Skip to content

fix: deduplicate managed PR comments created by concurrent Action runs#834

Merged
kitsuyui merged 1 commit into
mainfrom
fix/audit-io-atomicity-comment-toctou-001
May 25, 2026
Merged

fix: deduplicate managed PR comments created by concurrent Action runs#834
kitsuyui merged 1 commit into
mainfrom
fix/audit-io-atomicity-comment-toctou-001

Conversation

@kitsuyui
Copy link
Copy Markdown
Owner

What changed

Added post-create deduplication to updateMessage in src/github.ts to
clean up managed PR comments that accumulate when two Action runs race.

New helpers

  • getAllManagedComments() — fetches all matching managed comments (by
    user login + COMMENT_MARKER), sorted by id ascending.
  • deduplicateManagedComments() — calls getAllManagedComments, deletes
    every entry except the last (highest id), returns the survivor.
  • applyCommentAction() — extracted from updateMessage to keep
    cyclomatic complexity inside the Biome limit.

Changed behaviour

updateMessage now:

  1. Calls deduplicateManagedComments instead of getManagedComment.
    If a previous race left more than one managed comment, the stale
    duplicates are deleted before the current action is decided.
  2. After a successful createComment, calls deduplicateManagedComments
    again so any duplicate created in the same window is removed
    immediately.

Why

GitHub Issues API provides no conditional write mechanism (no If-Match
header). When two Action runs trigger on the same PR within milliseconds,
both call listComments, both see no managed comment, and both call
createComment. The second comment is then never updated or deleted by
future runs because find() always picks the first match.

This fix cannot prevent the race (the API does not allow it), but it
resolves the stale-comment state eagerly on every subsequent run.

Verification

  • bun run lint — no issues
  • bun run build — build complete
  • bun run test — 46 passed (added 2 new tests covering the
    deduplication and post-create cleanup paths)
  • madge --circular src/ — no circular dependency found

Trade-offs

  • The fix adds a second listComments call after createComment. This
    is a best-effort cleanup, not a hard guarantee; two runs that finish
    their creates and re-fetches in an interleaved order could both decide
    they are the winner and keep their own comment. In practice this window
    is much smaller than the original race, and any remaining duplicate is
    cleaned up on the very next run.
  • getAllManagedComments fetches up to 100 comments per call. PRs with
    more than 100 comments could miss older managed comments, but this
    matches the previous behaviour.

GitHub Issues API has no conditional write support (no If-Match), so
two concurrent runs that both see no managed comment will both call
createComment, leaving a stale duplicate that subsequent runs never
update or delete.

Add deduplicateManagedComments() which fetches all managed comments for
the user, deletes every one except the most-recently-created (highest
id), and returns the survivor. updateMessage now calls this before
deciding the action, and also calls it again after a successful
createComment so that any duplicate added in the same race window is
removed immediately.

Extract applyCommentAction() to keep cyclomatic complexity inside the
Biome limit and make the intent of each branch clearer.
@github-actions
Copy link
Copy Markdown

🎉 Happy commit!

@github-actions
Copy link
Copy Markdown

gh-counter

PR gate

Removed Added +/-
TODO/FIXME 0 0 0

Repo dashboard

main (7382600) #834 (03952c7) +/-
TODO/FIXME 0 0 0

Reported by gh-counter

@github-actions
Copy link
Copy Markdown

Code Metrics Report

main (7382600) #834 (03952c7) +/-
Coverage 99.1% 99.1% +0.0%
Code to Test Ratio 1:0.5 1:0.5 +0.0
Test Execution Time 1s 1s 0s
Details
  |                     | main (7382600) | #834 (03952c7) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          99.1% |          99.1% | +0.0% |
  |   Files             |              5 |              5 |     0 |
  |   Lines             |            115 |            122 |    +7 |
+ |   Covered           |            114 |            121 |    +7 |
+ | Code to Test Ratio  |          1:0.5 |          1:0.5 |  +0.0 |
  |   Code              |           1511 |           1598 |   +87 |
+ |   Test              |            834 |            898 |   +64 |
  | Test Execution Time |             1s |             1s |    0s |

Code coverage of files in pull request scope (100.0% → 100.0%)

Files Coverage +/- Status
src/github.ts 100.0% 0.0% modified

Reported by octocov

@github-actions
Copy link
Copy Markdown

gh-build-size

Target main #834 +/-
Total action artifacts 23,296,665 B 23,299,554 B +2,889 B
Runtime bundle 1,153,269 B 1,153,519 B +250 B
Source maps 17,127,738 B 17,130,010 B +2,272 B
Type declarations 4,973,589 B 4,973,956 B +367 B

Reported by gh-build-size

@kitsuyui kitsuyui merged commit fd2ef70 into main May 25, 2026
5 checks passed
@kitsuyui kitsuyui deleted the fix/audit-io-atomicity-comment-toctou-001 branch May 25, 2026 13:40
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.

1 participant