diff --git a/cmd/server/main.go b/cmd/server/main.go index 872c7d7..b562162 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -44,12 +44,16 @@ var gitSHA string // resolveCommitSHA returns the commit identifier for the deploy notification. // Coolify injects SOURCE_COMMIT into the container environment at runtime, so -// prefer it; fall back to the ldflags-baked gitSHA for local builds. +// prefer it; fall back to the ldflags-baked gitSHA for local builds; and when +// neither is set, report "unknown" so the owner still gets the startup DM. func resolveCommitSHA(envSourceCommit string) string { if s := strings.TrimSpace(envSourceCommit); s != "" { return s } - return gitSHA + if gitSHA != "" { + return gitSHA + } + return "unknown" } // factories is the static module catalog. Adding a new module is a one-line @@ -166,7 +170,6 @@ func main() { deploynotify.Run(rootCtx, deploynotify.Config{ Bot: b, - Store: deploynotify.NewStore(provider.Collection("deploynotify")), OwnerID: cfg.BotOwnerID, GitSHA: resolveCommitSHA(cfg.SourceCommit), }) diff --git a/cmd/server/main_test.go b/cmd/server/main_test.go index b1918d7..ba55f18 100644 --- a/cmd/server/main_test.go +++ b/cmd/server/main_test.go @@ -22,10 +22,10 @@ func TestResolveCommitSHA(t *testing.T) { t.Errorf("env empty: got %q, want baked fallback", got) } - // Neither source set → empty (deploynotify stays silent). + // Neither source set → "unknown" so the owner still gets the startup DM. gitSHA = "" - if got := resolveCommitSHA(" "); got != "" { - t.Errorf("both empty: got %q, want empty", got) + if got := resolveCommitSHA(" "); got != "unknown" { + t.Errorf("both empty: got %q, want unknown", got) } } diff --git a/internal/deploynotify/deploy_notify.go b/internal/deploynotify/deploy_notify.go index 5a89d6a..5889a10 100644 --- a/internal/deploynotify/deploy_notify.go +++ b/internal/deploynotify/deploy_notify.go @@ -1,40 +1,31 @@ -// Package deploynotify sends a one-shot Telegram DM to the bot owner when -// the binary starts up with a git SHA that hasn't been notified before. +// Package deploynotify sends a Telegram DM to the bot owner on every startup, +// announcing the running version (commit SHA). Mirrors the store-scraper-bot +// behaviour: fire once per boot, no dedup. The SHA comes from the SOURCE_COMMIT +// runtime env (Coolify injects it); when unknown the caller passes "unknown" +// rather than silencing the notice. // -// Wiring: cmd/server/main.go calls Run after modules.Install. The package- -// level gitSHA variable in main is populated via -ldflags at build time -// (see Makefile). An empty gitSHA — e.g. `go run` or a build without the -// ldflags — is treated as a signal to stay silent. -// -// Dedup: a single store record (key=last_notified_sha) holds the most recently -// notified SHA. On match we return early; on miss we send first, then write -// — so a transient Telegram failure doesn't permanently silence retries. +// Wiring: cmd/server/main.go calls Run after the webhook is cleared. Run is +// fire-and-forget — it never returns an error and never panics. package deploynotify import ( "context" - "errors" "fmt" "time" "github.com/go-telegram/bot" "github.com/tiennm99/miti99bot/internal/log" - "github.com/tiennm99/miti99bot/internal/storage" ) -// storeKey is the single store slot this package owns. -const storeKey = "last_notified_sha" - -// defaultTimeout caps the whole Run path (store read + Telegram send + store write) -// so a misbehaving network can never block Lambda init past its 10s budget. +// defaultTimeout caps the whole Run path (Telegram send) so a misbehaving +// network can never block process init for long. const defaultTimeout = 3 * time.Second // Config bundles the runtime dependencies. Sender is a seam for tests; when // nil, Run falls back to bot.SendMessage. type Config struct { Bot *bot.Bot - Store storage.DocStore[notifyRecord] OwnerID int64 GitSHA string Timeout time.Duration @@ -43,22 +34,8 @@ type Config struct { Sender func(ctx context.Context, chatID int64, text string) error } -// notifyRecord is the store value shape. At is informational only — useful for -// eyeballing in the database console; not consulted by the code path. -type notifyRecord struct { - SHA string `json:"sha" bson:"sha"` - At int64 `json:"at" bson:"at"` -} - -// NewStore builds the typed store this package needs from a module Collection. -// notifyRecord is unexported, so callers cannot name DocStore[notifyRecord] -// directly — this is the seam that lets cmd/server wire the store. -func NewStore(c storage.Collection) storage.DocStore[notifyRecord] { - return storage.Typed[notifyRecord](c) -} - -// Run is the entry point. Fire-and-forget — never returns an error and -// never panics. Designed to be called once during process init. +// Run sends the startup DM. Fire-and-forget — never returns an error and never +// panics. Sends on every call (no dedup): one DM per process start. func Run(ctx context.Context, cfg Config) { if reason := skipReason(cfg); reason != "" { log.Info("deploynotify skip", "reason", reason) @@ -71,66 +48,26 @@ func Run(ctx context.Context, cfg Config) { ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - notify, err := shouldNotify(ctx, cfg.Store, cfg.GitSHA) - if err != nil { - // Treat store errors as "not notified yet": worst case is one extra - // DM on the next cold start, which is far better than going - // silent on every deploy because the store threw a transient error. - log.Warn("deploynotify store read failed; will attempt send anyway", "err", err) - notify = true - } - if !notify { - return - } - if err := sendMessage(ctx, cfg, renderMessage(cfg.GitSHA)); err != nil { log.Warn("deploynotify telegram send failed", "err", err, "owner", cfg.OwnerID) return } - if err := markNotified(ctx, cfg.Store, cfg.GitSHA); err != nil { - log.Warn("deploynotify store write failed (owner was notified)", "err", err) - return - } log.Info("deploynotify sent", "sha", cfg.GitSHA, "owner", cfg.OwnerID) } // skipReason returns a non-empty short string when Run should no-op without -// touching the store or Telegram. Empty string ⇒ proceed. +// touching Telegram. Empty string ⇒ proceed. Note an empty/unknown SHA is NOT +// a skip reason — the owner still gets the startup notice. func skipReason(cfg Config) string { switch { - case cfg.GitSHA == "": - return "empty gitSHA (build without -ldflags)" case cfg.OwnerID == 0: return "no OWNER_ID configured" - case cfg.Store == nil: - return "no Store configured" case cfg.Bot == nil && cfg.Sender == nil: return "no bot or sender configured" } return "" } -// shouldNotify reports whether sha differs from the last notified value. -// A missing record (ErrNotFound) is treated as "yes, notify". -func shouldNotify(ctx context.Context, store storage.DocStore[notifyRecord], sha string) (bool, error) { - prev, _, err := store.Get(ctx, storeKey) - if errors.Is(err, storage.ErrNotFound) { - return true, nil - } - if err != nil { - return false, err - } - return prev.SHA != sha, nil -} - -// markNotified writes the SHA + current timestamp to the store. -func markNotified(ctx context.Context, store storage.DocStore[notifyRecord], sha string) error { - return store.Put(ctx, storeKey, notifyRecord{ - SHA: sha, - At: time.Now().UTC().UnixMilli(), - }) -} - // renderMessage is exposed for tests; keep the format stable enough that the // owner can grep their Telegram history by SHA. func renderMessage(sha string) string { diff --git a/internal/deploynotify/deploy_notify_test.go b/internal/deploynotify/deploy_notify_test.go index 6073f42..8f829c1 100644 --- a/internal/deploynotify/deploy_notify_test.go +++ b/internal/deploynotify/deploy_notify_test.go @@ -5,57 +5,10 @@ import ( "errors" "strings" "testing" - - "github.com/tiennm99/miti99bot/internal/storage" ) -// newNotifyStore returns a fresh in-memory typed notifyRecord store for tests. -func newNotifyStore() storage.DocStore[notifyRecord] { - return storage.Typed[notifyRecord](storage.NewMemoryProvider().Collection("deploynotify")) -} - -func TestShouldNotify_FirstRun(t *testing.T) { - store := newNotifyStore() - got, err := shouldNotify(context.Background(), store, "abc123") - if err != nil { - t.Fatalf("err: %v", err) - } - if !got { - t.Errorf("first run with empty store → want true, got false") - } -} - -func TestShouldNotify_SameSHA(t *testing.T) { - store := newNotifyStore() - if err := markNotified(context.Background(), store, "abc123"); err != nil { - t.Fatal(err) - } - got, err := shouldNotify(context.Background(), store, "abc123") - if err != nil { - t.Fatalf("err: %v", err) - } - if got { - t.Errorf("same SHA → want false, got true") - } -} - -func TestShouldNotify_DifferentSHA(t *testing.T) { - store := newNotifyStore() - if err := markNotified(context.Background(), store, "old111"); err != nil { - t.Fatal(err) - } - got, err := shouldNotify(context.Background(), store, "new222") - if err != nil { - t.Fatalf("err: %v", err) - } - if !got { - t.Errorf("changed SHA → want true, got false") - } -} - // recorder is a Sender that captures the last (chatID, text) it received and -// flags whether it was ever invoked. Replaces a full RecordingBot since this -// package only needs send-or-not signal. +// flags whether it was ever invoked. type recorder struct { called bool chatID int64 @@ -70,52 +23,15 @@ func (r *recorder) send(_ context.Context, chatID int64, text string) error { return r.err } -func TestRun_SkipsWhenSHAEmpty(t *testing.T) { - store := newNotifyStore() +func TestRun_SendsOnStartup(t *testing.T) { rec := &recorder{} Run(context.Background(), Config{ - Store: store, - OwnerID: 42, - GitSHA: "", - Sender: rec.send, - }) - if rec.called { - t.Errorf("empty SHA must not send; got call with text=%q", rec.text) - } - // And no store write either. - if _, _, err := store.Get(context.Background(), storeKey); !errors.Is(err, storage.ErrNotFound) { - t.Errorf("empty SHA must not write store; got err=%v", err) - } -} - -func TestRun_SkipsWhenNoOwner(t *testing.T) { - store := newNotifyStore() - rec := &recorder{} - Run(context.Background(), Config{ - Store: store, - OwnerID: 0, - GitSHA: "abc123", - Sender: rec.send, - }) - if rec.called { - t.Errorf("zero owner must not send") - } - if _, _, err := store.Get(context.Background(), storeKey); !errors.Is(err, storage.ErrNotFound) { - t.Errorf("zero owner must not write store; got err=%v", err) - } -} - -func TestRun_SendsAndPersistsOnFirstRun(t *testing.T) { - store := newNotifyStore() - rec := &recorder{} - Run(context.Background(), Config{ - Store: store, OwnerID: 42, GitSHA: "abc123", Sender: rec.send, }) if !rec.called { - t.Fatalf("first run with fresh store must send") + t.Fatal("startup must send the owner DM") } if rec.chatID != 42 { t.Errorf("chatID = %d, want 42", rec.chatID) @@ -123,48 +39,45 @@ func TestRun_SendsAndPersistsOnFirstRun(t *testing.T) { if !strings.Contains(rec.text, "abc123") { t.Errorf("message %q missing SHA", rec.text) } - // Store must now hold the SHA so the next Run is silent. - got, _, err := store.Get(context.Background(), storeKey) - if err != nil { - t.Fatalf("post-send store read: %v", err) - } - if got.SHA != "abc123" { - t.Errorf("persisted SHA = %q, want abc123", got.SHA) +} + +func TestRun_SendsEveryStartupNoDedup(t *testing.T) { + // Unlike the old dedup behaviour, the same SHA must notify on every boot. + for i := 0; i < 2; i++ { + rec := &recorder{} + Run(context.Background(), Config{OwnerID: 42, GitSHA: "abc123", Sender: rec.send}) + if !rec.called { + t.Fatalf("run %d: same SHA must still send (no dedup)", i) + } } } -func TestRun_SilentOnSecondRunSameSHA(t *testing.T) { - store := newNotifyStore() - if err := markNotified(context.Background(), store, "abc123"); err != nil { - t.Fatal(err) - } +func TestRun_SendsWithUnknownSHA(t *testing.T) { + // An unknown SHA is reported, not silenced. rec := &recorder{} - Run(context.Background(), Config{ - Store: store, - OwnerID: 42, - GitSHA: "abc123", - Sender: rec.send, - }) - if rec.called { - t.Errorf("repeat run with same SHA must not send") + Run(context.Background(), Config{OwnerID: 42, GitSHA: "unknown", Sender: rec.send}) + if !rec.called { + t.Fatal("unknown SHA must still send") + } + if !strings.Contains(rec.text, "unknown") { + t.Errorf("message %q should carry the unknown SHA", rec.text) } } -func TestRun_DoesNotPersistOnSendFailure(t *testing.T) { - store := newNotifyStore() - rec := &recorder{err: errors.New("telegram is down")} - Run(context.Background(), Config{ - Store: store, - OwnerID: 42, - GitSHA: "abc123", - Sender: rec.send, - }) - if !rec.called { - t.Fatalf("sender should have been called") +func TestRun_SkipsWhenNoOwner(t *testing.T) { + rec := &recorder{} + Run(context.Background(), Config{OwnerID: 0, GitSHA: "abc123", Sender: rec.send}) + if rec.called { + t.Error("zero owner must not send") } - // Send failed → SHA must NOT be persisted, so the next cold start retries. - if _, _, err := store.Get(context.Background(), storeKey); !errors.Is(err, storage.ErrNotFound) { - t.Errorf("failed send must not write store; got err=%v", err) +} + +func TestRun_SendFailureIsSwallowed(t *testing.T) { + rec := &recorder{err: errors.New("telegram is down")} + // Must not panic or block; just logs and returns. + Run(context.Background(), Config{OwnerID: 42, GitSHA: "abc123", Sender: rec.send}) + if !rec.called { + t.Fatal("sender should have been called") } }