Skip to content

fix(close): treat re-close of already-closed issue as idempotent success (GH#4025)#4113

Merged
maphew merged 1 commit into
gastownhall:mainfrom
kevglynn:fix/4025-close-idempotent
May 28, 2026
Merged

fix(close): treat re-close of already-closed issue as idempotent success (GH#4025)#4113
maphew merged 1 commit into
gastownhall:mainfrom
kevglynn:fix/4025-close-idempotent

Conversation

@kevglynn

Copy link
Copy Markdown
Contributor

Summary

Fixes #4025.

bd close <id> returns "issue not found" when re-closing an already-closed issue within the same wall-clock second. The root cause: the UPDATE touches no columns (status already closed, timestamps identical within the same second), so RowsAffected() returns 0 — previously misinterpreted as "row doesn't exist."

Fix

When RowsAffected == 0, query the row's current status in the same transaction:

  • If already closed → return success silently (idempotent, no duplicate event)
  • If row truly missing → preserve "issue not found" error
  • If row exists but in unexpected state → return generic failure

Follows the same idempotent pattern already used in claim.go.

Test plan

  • Close an issue, immediately close again → success (no error)
  • Close a nonexistent ID → still returns "issue not found"
  • No duplicate closed event recorded on re-close
  • Scripts doing bd close $id && bd close $id no longer fail

…ess (GH#4025)

When bd close is called on an issue already in closed state within the
same wall-clock second, the UPDATE touches no columns and RowsAffected
returns 0. Previously this was misinterpreted as "issue not found".

Now when RowsAffected==0, query the row's current status: if already
closed, return success silently (no duplicate event recorded). If the
row truly doesn't exist, preserve the original "issue not found" error.

Follows the same idempotent pattern used in claim.go.
@codecov-commenter

codecov-commenter commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
internal/storage/issueops/close.go 0.00% 11 Missing ⚠️

📢 Thoughts on this report? Let us know!

@harry-miller-trimble

Copy link
Copy Markdown
Collaborator

Thanks, @kevglynn! Rebased onto current main (this branch is ~50 commits behind) and added sqlmock tests for the new idempotent-close path in #4199. Your original commit is preserved there with attribution.

Continuing review in #4199; this can be closed in favor of it.

@maphew maphew merged commit 6d803c4 into gastownhall:main May 28, 2026
41 checks passed
@kevglynn

Copy link
Copy Markdown
Contributor Author

Thanks @harry-miller-trimble! The sqlmock tests for the idempotent path are a nice addition.

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.

bug: bd close <id> returns "issue not found" when re-closed within same wall-clock second (RowsAffected==0 misinterpreted as missing row)

4 participants