fix: Discord 分割送信の途中失敗時の重複投稿を防ぐ (#94)#97
Conversation
チャンク単位の last_read_at 付き一括既読化を導入。通知を updated_at 昇順に ソートし、各チャンクの送信成功ごとに送信済み分だけを PUT /notifications (last_read_at) で既読化する。途中失敗しても送信済み分は既読化済みのため、 未送信分だけが次回再取得され前半チャンクが重複投稿されない。 - Github::Notification に updated_at を追加 - notification_to_read に last_read_at パラメータを追加 - Notify::PostRepository#send_messages をチャンク送信成功ごとに累計件数を yield するブロック付き I/F に変更（Slack は atomic なので一度だけ yield） - Notify::Usecase が未送信先頭と同時刻の通知を巻き込まないよう last_read_at を 境界手前に丸める - Notify::Usecase の既読化ロジックを検証する spec を追加 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements chunk-by-chunk notification marking as read to prevent duplicate posts in case of partial failures during delivery. It introduces the updated_at field to GitHub notifications, sorts notifications chronologically, and updates the post repositories to yield cumulative sent counts so that the use case can safely advance the last_read_at timestamp. Feedback suggests adding a Content-Type: application/json header to the GitHub API request to ensure the JSON body is parsed correctly, and defensively clamping the sent count to prevent potential out-of-bounds index errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- notification_to_read の PUT /notifications に Content-Type: application/json を明示。無いと GitHub 側でボディが解析されず last_read_at が無視される恐れ - mark_read_through で sent_count を通知件数でクランプし、poster 実装（外部 境界）から想定外の値が渡っても安全にスライスできるようにする Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e68a9d3203
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
/notifications は既定で 1 ページ分しか返さないため、一部ページのみ取得した 状態で last_read_at 既読化を行うと、未取得の古い通知（更新時刻が小さく チャンク送信対象外）まで last_read_at 以前として既読化され、未送信のまま 消える恐れがあった。全ページ取得してから境界を決めるよう修正する。 - find_notifications_unread を per_page=100 で全ページ取得に変更 - 途中ページの一時失敗（5xx/401）時は不完全な取得での既読化を避けるため その実行を丸ごとスキップし次回に委ねる - 暴走防止に MAX_PAGES=20 の上限を設け、到達時はログを残す Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b061d9510
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- ページ数上限(MAX_PAGES)到達で取得しきれない場合は不完全取得としてその実行を スキップ。未取得の古い通知を last_read_at で巻き込み既読化するのを防ぐ - ページング中の新着によるオフセットずれで通知が抜けるのを防ぐため、取得開始 時刻を before に固定してページ集合を安定させる - notification_to_read が PUT のレスポンスを検証し、既読化失敗時は例外にして 後続チャンクの送信を止める（重複防止は既読化成功に依存するため） - 既読化失敗時に送信を止めることを検証する spec を追加 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
概要
issue #94 を解決する。Discord モードで通知が複数の webhook 投稿に分割される際、分割送信の途中で失敗すると前半チャンクが送信済みなのに既読化されず、次回実行で重複投稿されうる問題を修正する。
issue で検討された チャンク単位の
last_read_at付き一括既読化 を採用した（スレッド単位既読化の N+1 懸念を避けつつ、既読化コールを送信コールと同数に抑えられる案）。変更内容
updated_at昇順にソートしてから送信 (Notify::Usecase)Notify::PostRepository#send_messagesを、チャンク送信成功ごとに累計送信件数をyieldするブロック付き I/F に変更Notify::Usecaseが yield された件数からlast_read_atを算出しPUT /notificationsを都度呼ぶGitHub::NotificationRepository#notification_to_readにlast_read_atパラメータを追加Github::Notificationにupdated_atを追加途中失敗時の挙動
チャンク送信が途中で失敗しても、送信済みチャンクまでは
last_read_atで既読化済みのため、未送信分だけが次回実行で再取得される。前半チャンクの重複投稿が原理的に発生しない。同一タイムスタンプの境界考慮
issue の注意点どおり、未送信の先頭通知と同一
updated_atの通知まで巻き込んで既読化しないよう、last_read_atを「未送信先頭より前の最大updated_at」に丸めている。安全に前進できる位置が無い（送信済みが全て未送信先頭と同時刻）稀なケースはスキップし、次回実行に委ねる（通知ロストより稀な重複を許容）。副次効果
現行のパラメータなし
PUT /notifications（現在時刻までの全既読化）にあった「fetch 後〜既読化までに届いた新規通知を通知せず既読化する窓」も、last_read_at方式により狭まる。テスト
spec/notify/usecase_spec.crを新規追加し、全チャンク成功 / 途中失敗 / 同一タイムスタンプ境界 / 通知ゼロ件の既読化挙動を検証crystal spec31 examples 0 failures /ameba0 failures /crystal build src/main.cr成功Closes #94
🤖 Generated with Claude Code