# 283 SlackのDM送信を5分ごとに実行する処理の追加#322
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAPI 起動時の context 伝播を追加し、Echo サーバの停止処理を context ベースに変更しました。あわせて汎用スケジューラを新規追加し、DI 層で Slack 通知の定期実行を 5 分間隔で開始するよう配線しています。AGENTS.md には関連する構成と運用前提を追記しました。 Changes通知定期実行スケジューラ
推定コードレビュー工数🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
AGENTS.md (1)
52-56: ⚡ Quick winGraceful shutdown 未対応の制約をドキュメント化してください
現在の実装では scheduler の graceful shutdown が未対応です（PR objectives でも明示的に defer されています）。将来の開発者のため、この制約を「Known Transitional Issues」セクションなどに記載することを推奨します。
📝 ドキュメント追記案
Lines 188-196 の「Known Transitional Issues」セクションに以下を追加：
- **Scheduler の graceful shutdown**: 現在 `externals/scheduler` は Context cancellation に未対応。サーバーシャットダウン時に goroutine が即座に停止しない → 将来対応予定🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` around lines 52 - 56, The scheduler implementation currently lacks graceful shutdown support, but this limitation is not documented. Add a note to the "Known Transitional Issues" section around lines 188-196 in AGENTS.md to explicitly document that the externals/scheduler does not support Context cancellation, meaning goroutines may not stop immediately during server shutdown. This will alert future developers to this constraint and its status as a deferred item for future implementation.api/lib/externals/scheduler/scheduler.go (1)
25-37: ⚡ Quick win起動直後の即時実行を検討してください
現在の実装では ticker の最初の tick（5分後）まで job が実行されません。サーバー起動時に未送信通知が溜まっている場合、最大5分間送信が遅延します。起動直後に一度 job を実行してから定期実行に入る設計も検討できます。
♻️ 起動直後の即時実行案
func (s *Scheduler) Start(ctx context.Context) { go func() { + // 起動直後に一度実行 + if err := s.job(ctx); err != nil { + log.Printf("[scheduler:%s] job error (initial): %v", s.name, err) + } + ticker := time.NewTicker(s.interval) defer ticker.Stop() for { select { case <-ticker.C: if err := s.job(ctx); err != nil { log.Printf("[scheduler:%s] job error: %v", s.name, err) } case <-ctx.Done(): log.Printf("[scheduler:%s] stopped: %v", s.name, ctx.Err()) return } } }() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/lib/externals/scheduler/scheduler.go` around lines 25 - 37, The Start method currently waits for the first ticker tick before executing the job, causing delays in processing pending notifications on startup. Add an immediate execution of s.job(ctx) right after creating the ticker and before the for range ticker.C loop begins. This ensures the job runs immediately when Start is called, then continues with the regular interval-based execution in the ticker loop.api/lib/di/di.go (1)
102-105: 💤 Low valueSLACK_BOT_TOKEN 未設定時のエラーハンドリング
NewSlackService()が失敗するとlog.Fatalfでサーバー起動が中断されます。通知機能がコア機能であれば fail-fast は適切ですが、開発環境で Slack 連携なしで起動したい場合は不便です。開発時は Slack 連携をオプショナルにする（scheduler を起動しない）設計も検討できます。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/lib/di/di.go` around lines 102 - 105, The current implementation uses log.Fatalf in the Slack service initialization within the NewSlackService() call, which crashes the entire server if Slack token is not configured. Instead of failing hard, make Slack integration optional by logging a warning when NewSlackService() fails and conditionally skip scheduler startup when the slack service is unavailable. This allows the application to start in development environments without requiring Slack configuration, while still enabling Slack functionality when the token is properly set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/lib/di/di.go`:
- Around line 101-111: The scheduler's Start() method does not properly handle
context cancellation, preventing graceful shutdown. First, modify the ticker
loop in scheduler.go's Start() method to include a select statement that checks
for context cancellation using <-ctx.Done(), ensuring the scheduler stops when
the context is cancelled. Second, replace the context.Background() call in the
di.go initialization with a cancellable context that is properly propagated from
signal handling logic (add signal handling in main.go or server.RunServer to
catch SIGTERM and SIGINT), allowing the scheduler to be gracefully stopped when
the application receives shutdown signals.
In `@api/lib/externals/scheduler/scheduler.go`:
- Around line 25-37: The Start method's goroutine does not respond to Context
cancellation, causing goroutine leaks and preventing graceful shutdown. Modify
the for loop to use a select statement that monitors both ticker.C for job
execution and ctx.Done() for cancellation signals. When the context is cancelled
(ctx.Done() receives a signal), the function should break the loop, allowing the
defer ticker.Stop() to execute and properly clean up resources. This ensures the
goroutine exits when the context is cancelled rather than running indefinitely.
---
Nitpick comments:
In `@AGENTS.md`:
- Around line 52-56: The scheduler implementation currently lacks graceful
shutdown support, but this limitation is not documented. Add a note to the
"Known Transitional Issues" section around lines 188-196 in AGENTS.md to
explicitly document that the externals/scheduler does not support Context
cancellation, meaning goroutines may not stop immediately during server
shutdown. This will alert future developers to this constraint and its status as
a deferred item for future implementation.
In `@api/lib/di/di.go`:
- Around line 102-105: The current implementation uses log.Fatalf in the Slack
service initialization within the NewSlackService() call, which crashes the
entire server if Slack token is not configured. Instead of failing hard, make
Slack integration optional by logging a warning when NewSlackService() fails and
conditionally skip scheduler startup when the slack service is unavailable. This
allows the application to start in development environments without requiring
Slack configuration, while still enabling Slack functionality when the token is
properly set.
In `@api/lib/externals/scheduler/scheduler.go`:
- Around line 25-37: The Start method currently waits for the first ticker tick
before executing the job, causing delays in processing pending notifications on
startup. Add an immediate execution of s.job(ctx) right after creating the
ticker and before the for range ticker.C loop begins. This ensures the job runs
immediately when Start is called, then continues with the regular interval-based
execution in the ticker loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 639b2fd0-53b1-4e28-b2ad-41cdfa415504
📒 Files selected for processing (3)
AGENTS.mdapi/lib/di/di.goapi/lib/externals/scheduler/scheduler.go
- ticker の初回 tick（5分後）を待たず、起動時に一度 ProcessUnsentNotifications を実行 - 再起動直後に溜まっていた未送信通知の flush 遅延（最大5分）を解消
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/lib/di/di.go (1)
102-105: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSlack未設定の環境でも API を起動できるようにする
slack.NewSlackService()が失敗するとlog.FatalfでInitializeServer全体が止まります。SLACK_BOT_TOKENがない環境でも API 本体を立ち上げたいなら、通知 scheduler を無効化するか warning で継続する fallback にしてください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/lib/di/di.go` around lines 102 - 105, The Slack service initialization in InitializeServer currently hard-fails via slack.NewSlackService() and log.Fatalf, which stops the API when Slack is unavailable. Update this flow so a missing or invalid Slack configuration only disables the notification scheduler or logs a warning and continues startup, using the slack.NewSlackService and InitializeServer symbols to locate the fallback path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/lib/externals/server/server.go`:
- Around line 45-58: RunServer is terminating the process via e.Logger.Fatal
instead of returning errors, which prevents callers from handling cleanup.
Update RunServer to propagate failures from e.Start and e.Shutdown as returned
errors, and keep the server startup/shutdown flow in that function so the caller
can decide how to log or exit. Use the existing RunServer, e.Start, and
e.Shutdown symbols to replace the Fatal calls with error returns and preserve
deferred cleanup behavior in the caller.
---
Outside diff comments:
In `@api/lib/di/di.go`:
- Around line 102-105: The Slack service initialization in InitializeServer
currently hard-fails via slack.NewSlackService() and log.Fatalf, which stops the
API when Slack is unavailable. Update this flow so a missing or invalid Slack
configuration only disables the notification scheduler or logs a warning and
continues startup, using the slack.NewSlackService and InitializeServer symbols
to locate the fallback path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 78b57f3e-3b36-41c3-96f1-77a2a88922a4
📒 Files selected for processing (4)
api/lib/di/di.goapi/lib/externals/scheduler/scheduler.goapi/lib/externals/server/server.goapi/main.go
🚧 Files skipped from review as they are similar to previous changes (1)
- api/lib/externals/scheduler/scheduler.go
- Fatal（os.Exit）は defer を飛ばすため、Start/Shutdown 失敗時に CloseDB 等が走らない問題を解消 - RunServer は error を返し、起動失敗は errCh 経由で受け取る形に - InitializeServer の戻り値を (db.Client, error) にし、db/slack 初期化失敗も伝播 - main を run() error パターンにし、defer 実行後に log.Fatal するよう変更
…ular-execution-of-notif
1 participant
対応Issue
resolve #271
概要
未送信ログを送信する
NotificationUseCase.ProcessUnsentNotificationsを、API プロセス内で5分間隔に自動実行する scheduler を追加した。これまでは
cmd/send-notificationsを手動で実行した瞬間だけ通知が処理され、action_logsにis_sent=falseのログが溜まり続けていた。このPRで、API を起動しておくだけで未送信ログが5分ごとに自動で Slack へ送られるようになる。#254 の手動実行を自動化したものにあたる。設計は3つのファイルに分かれる。
externals/schedulerは新規ファイルで、ticker ループで Job を goroutine として回す。Job をfunc(ctx) errorの関数型で受け取ることで、scheduler から usecase を import せずに済み、疎結合を保てるdi.goでNotificationUseCaseを組み立て、server.RunServerの直前にscheduler.New("notification", 5*time.Minute, uc.ProcessUnsentNotifications).Start(ctx)を呼ぶ。Start は goroutine を起動してすぐ return するAGENTS.mdに、API を単一インスタンス前提で動かすことを明記したテスト項目
事前準備として次を行う。
SLACK_BOT_TOKENを env に設定する。値は Slack App の OAuth ページで取得できるusers.slack_user_idに自分の Slack member ID を設定する。ここが空だと DM の送り先が無いmake upで API と DB を起動する。Mac ではmac-upを使うそのうえで、自動送信を確認する。ここがこのPRの主眼になる。
action_logsにis_sent=falseのログを1件作るaction_logsのis_sentがtrueへ更新されていることを確認するdi.goの5*time.Minuteを一時的に30*time.Secondなどへ下げて周期を素早く確認し、確認後に戻すdocker exec -it nutfes-seeft-api sh -c "cd /app && go run ./cmd/send-notifications"が引き続き動くことも確認する備考
context.WithCancelを渡せば後付けできるSummary by CodeRabbit
リリースノート