From 4753ac8571440103f8a558def8f7ed6c47ba558d Mon Sep 17 00:00:00 2001 From: JonghunYu Date: Thu, 27 Nov 2025 10:36:19 +0900 Subject: [PATCH 1/3] chore: add 018 --- .issues/.counter | 2 +- .issues/closed/.keep | 0 .issues/open/.keep | 0 .issues/open/018-increment-counter.md | 43 +++++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 .issues/closed/.keep create mode 100644 .issues/open/.keep create mode 100644 .issues/open/018-increment-counter.md diff --git a/.issues/.counter b/.issues/.counter index 3c03207..d6b2404 100644 --- a/.issues/.counter +++ b/.issues/.counter @@ -1 +1 @@ -18 +19 diff --git a/.issues/closed/.keep b/.issues/closed/.keep new file mode 100644 index 0000000..e69de29 diff --git a/.issues/open/.keep b/.issues/open/.keep new file mode 100644 index 0000000..e69de29 diff --git a/.issues/open/018-increment-counter.md b/.issues/open/018-increment-counter.md new file mode 100644 index 0000000..c32f97b --- /dev/null +++ b/.issues/open/018-increment-counter.md @@ -0,0 +1,43 @@ +--- +id: "018" +assignee: "" +labels: [] +created: 2025-11-27T10:01:37.532453+09:00 +updated: 2025-11-27T10:01:37.532453+09:00 +--- + +# increment counter + +## Description + +When creating a new issue, if the counter points to an ID that already exists in the closed directory, the `gi create` command fails with an error instead of automatically finding the next available ID. + +**Current behavior:** +``` +$ gi create aa +Error: failed to save issue: issue 003 exists in closed directory, cannot save to open +``` + +This happens because `GetNextID()` in `pkg/storage.go` simply reads the counter value and increments it, without checking if that ID is already occupied by a closed issue. + +**Expected behavior:** +The system should automatically skip occupied IDs and use the next available one, updating the counter accordingly. + +## Requirements + +- Modify `GetNextID()` function in `pkg/storage.go:98` to check if the current counter ID exists +- If ID exists in either open or closed directory, increment and check again until an available ID is found +- Update the counter file to reflect the next available ID +- Maintain backward compatibility with existing counter behavior +- Handle edge cases (e.g., large gaps in ID sequence) + +## Success Criteria + +- [ ] `gi create` successfully creates issues even when counter points to closed issue ID +- [ ] Counter automatically increments to next available ID +- [ ] System handles multiple sequential occupied IDs correctly +- [ ] No breaking changes to existing functionality +- [ ] Unit tests cover the auto-increment logic +- [ ] Works correctly in both scenarios: + - Counter = 3, issue 003 in closed → creates issue 004 + - Counter = 5, issues 005-007 in closed → creates issue 008 From 20f3565a566a7fcef48adf68fa3a6d3b992964c4 Mon Sep 17 00:00:00 2001 From: JonghunYu Date: Thu, 27 Nov 2025 10:46:55 +0900 Subject: [PATCH 2/3] chore: close 018 --- .issues/open/018-increment-counter.md | 25 ++++-- pkg/storage.go | 21 +++++- pkg/storage_test.go | 105 ++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 10 deletions(-) diff --git a/.issues/open/018-increment-counter.md b/.issues/open/018-increment-counter.md index c32f97b..5b0b64b 100644 --- a/.issues/open/018-increment-counter.md +++ b/.issues/open/018-increment-counter.md @@ -33,11 +33,24 @@ The system should automatically skip occupied IDs and use the next available one ## Success Criteria -- [ ] `gi create` successfully creates issues even when counter points to closed issue ID -- [ ] Counter automatically increments to next available ID -- [ ] System handles multiple sequential occupied IDs correctly -- [ ] No breaking changes to existing functionality -- [ ] Unit tests cover the auto-increment logic -- [ ] Works correctly in both scenarios: +- [x] `gi create` successfully creates issues even when counter points to closed issue ID +- [x] Counter automatically increments to next available ID +- [x] System handles multiple sequential occupied IDs correctly +- [x] No breaking changes to existing functionality +- [x] Unit tests cover the auto-increment logic +- [x] Works correctly in both scenarios: - Counter = 3, issue 003 in closed → creates issue 004 - Counter = 5, issues 005-007 in closed → creates issue 008 + +## Implementation Notes + +Fixed in `pkg/storage.go:97-132`. The `GetNextID()` function now: +1. Checks if the current counter value points to an existing issue +2. Loops through IDs until it finds an available one +3. Updates the counter to the next available ID + +Added comprehensive unit tests in `pkg/storage_test.go`: +- `TestGetNextIDSkipsOccupiedIDs` - verifies skipping sequential occupied IDs +- `TestGetNextIDWithGapsInSequence` - verifies finding gaps in ID sequence + +All tests passing with 77.5% code coverage. diff --git a/pkg/storage.go b/pkg/storage.go index b881a3c..561787a 100644 --- a/pkg/storage.go +++ b/pkg/storage.go @@ -94,7 +94,7 @@ Describe the issue here... return nil } -// GetNextID reads and increments the counter +// GetNextID reads and increments the counter, skipping any IDs that already exist func GetNextID() (int, error) { counterPath := filepath.Join(IssuesDir, CounterFile) @@ -109,13 +109,26 @@ func GetNextID() (int, error) { return 0, fmt.Errorf("invalid counter value: %w", err) } - // Write incremented value - nextID := currentID + 1 + // Find the next available ID by checking if current ID exists + availableID := currentID + for { + formattedID := FormatID(availableID) + _, _, err := FindIssueFile(formattedID) + if err != nil { + // ID not found, so it's available + break + } + // ID exists (in either open or closed), try next one + availableID++ + } + + // Write the next ID after the one we're returning + nextID := availableID + 1 if err := os.WriteFile(counterPath, []byte(fmt.Sprintf("%d\n", nextID)), 0644); err != nil { return 0, fmt.Errorf("failed to write counter: %w", err) } - return currentID, nil + return availableID, nil } // SaveIssue writes an issue to the specified directory (open or closed) diff --git a/pkg/storage_test.go b/pkg/storage_test.go index 7010e62..0557077 100644 --- a/pkg/storage_test.go +++ b/pkg/storage_test.go @@ -116,6 +116,111 @@ func TestGetNextID(t *testing.T) { } } +func TestGetNextIDSkipsOccupiedIDs(t *testing.T) { + cleanup := setupTestRepo(t) + defer cleanup() + + if err := InitializeRepo(); err != nil { + t.Fatal(err) + } + + // Create and save issues with IDs 1, 2, 3 + issue1 := NewIssue(1, "First Issue", "", nil) + issue2 := NewIssue(2, "Second Issue", "", nil) + issue3 := NewIssue(3, "Third Issue", "", nil) + + if err := SaveIssue(issue1, OpenDir); err != nil { + t.Fatal(err) + } + if err := SaveIssue(issue2, ClosedDir); err != nil { + t.Fatal(err) + } + if err := SaveIssue(issue3, ClosedDir); err != nil { + t.Fatal(err) + } + + // Set counter to 1 (which is occupied in open directory) + counterPath := filepath.Join(IssuesDir, CounterFile) + if err := os.WriteFile(counterPath, []byte("1\n"), 0644); err != nil { + t.Fatal(err) + } + + // GetNextID should skip 1, 2, 3 and return 4 + id, err := GetNextID() + if err != nil { + t.Fatalf("GetNextID() error = %v", err) + } + if id != 4 { + t.Errorf("GetNextID() = %d, want 4 (should skip occupied IDs 1-3)", id) + } + + // Verify counter was updated to 5 + data, err := os.ReadFile(counterPath) + if err != nil { + t.Fatal(err) + } + if strings.TrimSpace(string(data)) != "5" { + t.Errorf("Counter = %q, want %q", strings.TrimSpace(string(data)), "5") + } +} + +func TestGetNextIDWithGapsInSequence(t *testing.T) { + cleanup := setupTestRepo(t) + defer cleanup() + + if err := InitializeRepo(); err != nil { + t.Fatal(err) + } + + // Create issues with gaps: 1, 3, 5 (missing 2, 4) + issue1 := NewIssue(1, "First Issue", "", nil) + issue3 := NewIssue(3, "Third Issue", "", nil) + issue5 := NewIssue(5, "Fifth Issue", "", nil) + + if err := SaveIssue(issue1, ClosedDir); err != nil { + t.Fatal(err) + } + if err := SaveIssue(issue3, OpenDir); err != nil { + t.Fatal(err) + } + if err := SaveIssue(issue5, ClosedDir); err != nil { + t.Fatal(err) + } + + // Set counter to 1 + counterPath := filepath.Join(IssuesDir, CounterFile) + if err := os.WriteFile(counterPath, []byte("1\n"), 0644); err != nil { + t.Fatal(err) + } + + // GetNextID should find the first gap at ID 2 + id, err := GetNextID() + if err != nil { + t.Fatalf("GetNextID() error = %v", err) + } + if id != 2 { + t.Errorf("GetNextID() = %d, want 2 (should find first gap)", id) + } + + // Next call should find ID 4 + id, err = GetNextID() + if err != nil { + t.Fatalf("GetNextID() error = %v", err) + } + if id != 4 { + t.Errorf("GetNextID() = %d, want 4 (should find second gap)", id) + } + + // Next call should skip 5 and return 6 + id, err = GetNextID() + if err != nil { + t.Fatalf("GetNextID() error = %v", err) + } + if id != 6 { + t.Errorf("GetNextID() = %d, want 6 (should skip occupied ID 5)", id) + } +} + func TestSaveAndLoadIssue(t *testing.T) { cleanup := setupTestRepo(t) defer cleanup() From c2acf8c3bb9ff1a49d07556a0d2239eaaacaee72 Mon Sep 17 00:00:00 2001 From: JonghunYu Date: Thu, 27 Nov 2025 10:51:36 +0900 Subject: [PATCH 3/3] chore: added manual test instruction --- .gitignore | 2 ++ CLAUDE.md | 3 +++ 2 files changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index e6a48fb..3b1d510 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,5 @@ go.work.sum # Editor/IDE .claude/ + +test/ diff --git a/CLAUDE.md b/CLAUDE.md index 9fbe36a..9dab116 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -97,6 +97,7 @@ golangci-lint run ### Cross-Platform Builds The Makefile should support: + ```bash make build # Current platform make build-all # macOS (ARM64/AMD64), Linux (AMD64) @@ -136,6 +137,7 @@ make lint # Run golangci-lint All commands use Cobra. Global flag: `-h, --help` **Command-specific flags:** + - `create`: `--assignee `, `--label