# 283 SlackのDM送信を5分ごとに実行する処理の追加#322
Open
taminororo wants to merge 6 commits into
Open
Conversation
📝 WalkthroughWalkthrough汎用スケジューラパッケージ Changes通知定期実行スケジューラ
推定コードレビュー工数🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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
Comment on lines
+101
to
+111
| // Scheduler: 5分間隔で未送信通知を flush する（goroutine で起動し即 return） | ||
| slackService, err := slack.NewSlackService() | ||
| if err != nil { | ||
| log.Fatalf("slack init: %v", err) | ||
| } | ||
| notificationUseCase := usecase.NewNotificationUseCase( | ||
| actionLogRepository, slackService, | ||
| userRepository, dateRepository, timeRepository, | ||
| taskRepository, shiftRepository, weatherRepository, | ||
| ) | ||
| scheduler.New("notification", 5*time.Minute, notificationUseCase.ProcessUnsentNotifications).Start(context.Background()) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# InitializeServer の呼び出し元を確認
rg -nP '\bInitializeServer\s*\(' --type=go -C3Repository: NUTFes/SeeFT
Length of output: 519
🏁 Script executed:
#!/bin/bash
# scheduler.go の実装を確認
fd -e go -p scheduler | head -20Repository: NUTFes/SeeFT
Length of output: 97
🏁 Script executed:
#!/bin/bash
# scheduler.go の内容を確認
find . -name "scheduler.go" -type f | xargs cat -n | head -100Repository: NUTFes/SeeFT
Length of output: 1150
🏁 Script executed:
#!/bin/bash
# main.go の全体を確認
cat -n api/main.goRepository: NUTFes/SeeFT
Length of output: 807
🏁 Script executed:
#!/bin/bash
# signal handling や graceful shutdown のパターンを検索
rg -i "signal|shutdown|cancel|interrupt" --type=go -C2 | head -150Repository: NUTFes/SeeFT
Length of output: 38
🏁 Script executed:
#!/bin/bash
# ProcessUnsentNotifications のシグネチャを確認
rg -nP 'ProcessUnsentNotifications.*\(' --type=go -A5 | head -40Repository: NUTFes/SeeFT
Length of output: 1653
🏁 Script executed:
#!/bin/bash
# scheduler の Job 型の使用箇所を確認
rg -nP 'scheduler.New|type Job' --type=go -B2 -A2Repository: NUTFes/SeeFT
Length of output: 738
scheduler の Start() メソッドが context キャンセルに対応していないため、graceful shutdown が不可能
scheduler.go の Start() メソッド（27-36行）は、渡された context を job に渡すだけで、スケジューラー自体が context のキャンセルをチェック（<-ctx.Done()）していません。そのため、context.Background() 以外の cancellable context を渡しても、ticker は無限に動作し続けます。
さらに、main.go にはシグナルハンドリングや graceful shutdown の仕組みがなく、サーバー停止時に scheduler を適切に停止できません。
修正が必要な箇所:
- scheduler.go の
Start()内の ticker ループに、context キャンセルをチェックする select 文を追加 - main.go（または server.RunServer）にシグナルハンドリングと context 伝播の仕組みを実装
🤖 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 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.
Comment on lines
+25
to
+37
| // Start は ticker ループを goroutine で起動し、即座に return する。 | ||
| // interval ごとに job を実行し、job が返したエラーはログ出力のみ（ループは止めない）。 | ||
| func (s *Scheduler) Start(ctx context.Context) { | ||
| go func() { | ||
| ticker := time.NewTicker(s.interval) | ||
| defer ticker.Stop() | ||
| for range ticker.C { | ||
| if err := s.job(ctx); err != nil { | ||
| log.Printf("[scheduler:%s] job error: %v", s.name, err) | ||
| } | ||
| } | ||
| }() | ||
| } |
There was a problem hiding this comment.
Context のキャンセルに対応していない（goroutine リーク）
for range ticker.C は Context がキャンセルされても終了しないため、goroutine がリークします。サーバーのシャットダウン時に ticker が停止せず、graceful shutdown が実装できません。
🔒 Context キャンセル対応の修正案
func (s *Scheduler) Start(ctx context.Context) {
go func() {
ticker := time.NewTicker(s.interval)
defer ticker.Stop()
- for range ticker.C {
- if err := s.job(ctx); err != nil {
- log.Printf("[scheduler:%s] job error: %v", s.name, err)
- }
+ 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
+ }
}
}()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Start は ticker ループを goroutine で起動し、即座に return する。 | |
| // interval ごとに job を実行し、job が返したエラーはログ出力のみ（ループは止めない）。 | |
| func (s *Scheduler) Start(ctx context.Context) { | |
| go func() { | |
| ticker := time.NewTicker(s.interval) | |
| defer ticker.Stop() | |
| for range ticker.C { | |
| if err := s.job(ctx); err != nil { | |
| log.Printf("[scheduler:%s] job error: %v", s.name, err) | |
| } | |
| } | |
| }() | |
| } | |
| // Start は ticker ループを goroutine で起動し、即座に return する。 | |
| // interval ごとに job を実行し、job が返したエラーはログ出力のみ（ループは止めない）。 | |
| func (s *Scheduler) Start(ctx context.Context) { | |
| go func() { | |
| 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'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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
対応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
リリースノート