diff --git a/plans/260522-1109-deploy-notify-owner/phase-01-implement-deploynotify-package-main-go-hook.md b/plans/260522-1109-deploy-notify-owner/phase-01-implement-deploynotify-package-main-go-hook.md new file mode 100644 index 0000000..7c0d814 --- /dev/null +++ b/plans/260522-1109-deploy-notify-owner/phase-01-implement-deploynotify-package-main-go-hook.md @@ -0,0 +1,141 @@ +--- +phase: 1 +title: Implement deploynotify package + main.go hook +status: completed +priority: P3 +effort: 1h +dependencies: [] +--- + +# Phase 1: Implement deploynotify package + main.go hook + +## Overview + +Create `internal/deploynotify/` — a small package that compares the +baked-in `gitSHA` against a `last_notified_sha` value in KV and DMs the +bot owner if (and only if) it changed. Wire it from `cmd/server/main.go` +right after `modules.Install`. + +## Requirements + +**Functional** +- Send exactly one Telegram DM to `BOT_OWNER_ID` per *new* gitSHA observed. +- On subsequent cold starts with the same SHA, send nothing. +- Skip silently when: `gitSHA` empty (local build), `BOT_OWNER_ID == 0`, + or KV operation fails. + +**Non-functional** +- Never panic; never return an error that aborts startup. +- ≤3s wall time on the happy path (network DM + 1 KV read + 1 KV write). +- No new env vars, no new IAM permissions (DynamoDB read/write already + granted to the partition). + +## Architecture + +``` +cmd/server/main.go + │ + │ after modules.Install(b, reg, auth): + ├─→ deploynotify.Run(ctx, deploynotify.Config{ + │ Bot: b, + │ KV: provider.For("deploynotify"), + │ OwnerID: cfg.BotOwnerID, + │ GitSHA: gitSHA, // package-level var, ldflags-injected + │ Timeout: 3 * time.Second, + │ }) + │ │ + │ ├─ skipReason() short-circuit (no SHA / no owner) + │ ├─ kv.GetJSON("last_notified_sha", &prev) + │ ├─ if prev.SHA == gitSHA → return (silent) + │ ├─ bot.SendMessage(owner, "🚀 miti99bot deployed: SHA") + │ └─ kv.PutJSON("last_notified_sha", {SHA: gitSHA, At: now}) +``` + +KV namespace: `deploynotify` (new partition, isolated from module data). +Key: `last_notified_sha`. +Value shape: +```go +type notifyRecord struct { + SHA string `json:"sha"` + At int64 `json:"at"` // ms-since-epoch, for debug only +} +``` + +Telegram message (plain text — no parse_mode dependency on formatting +edge cases): +``` +🚀 miti99bot deployed: +``` + +## Related Code Files + +- Create: `internal/deploynotify/deploy_notify.go` (snake_case per Go conventions) +- Create: `internal/deploynotify/deploy_notify_test.go` +- Modify: `cmd/server/main.go` — declare `var gitSHA string`, add + `deploynotify.Run(...)` call after `modules.Install(b, reg, auth)` and + before the `go func() { srv.ListenAndServe() }()` block. + +## Implementation Steps + +1. **Create `internal/deploynotify/deploy_notify.go`** with: + - `type Config struct { Bot *bot.Bot; KV storage.KVStore; OwnerID int64; GitSHA, Timeout }`. + - `func Run(ctx context.Context, cfg Config)` — fire-and-forget, no + error return; all failures logged via `internal/log`. + - Internal helper `shouldNotify(ctx, kv, sha) (bool, error)` so dedup + is unit-testable without a real Telegram bot. + - Internal helper `markNotified(ctx, kv, sha) error`. + - Internal `renderMessage(sha string) string` — single line, easy to test. + +2. **Wire into `cmd/server/main.go`**: + - Add `var gitSHA string` at package level (alongside `factories()`). + - After `modules.Install(b, reg, auth)` and the existing `log.Info("modules loaded", ...)`: + ```go + deploynotify.Run(rootCtx, deploynotify.Config{ + Bot: b, + KV: provider.For("deploynotify"), + OwnerID: cfg.BotOwnerID, + GitSHA: gitSHA, + Timeout: 3 * time.Second, + }) + ``` + - Import: `"github.com/tiennm99/miti99bot/internal/deploynotify"`. + +3. **Tests** (`deploy_notify_test.go`): + - `TestShouldNotify_FirstRun` — empty KV → returns true. + - `TestShouldNotify_SameSHA` — KV holds current SHA → returns false. + - `TestShouldNotify_DifferentSHA` — KV holds old SHA → returns true. + - `TestRun_SkipsWhenSHAEmpty` — gitSHA="" → no KV access, no send. + - `TestRun_SkipsWhenNoOwner` — OwnerID=0 → no KV access, no send. + - `TestRenderMessage_ContainsSHA` — output includes the SHA. + - Use `storage.NewMemoryKVStore()` for KV. + - For the Telegram send path: skip end-to-end Telegram tests — the + existing `testutil.RecordingBot` pattern is heavier than needed. + Cover send via an indirection: `Config.sender` field of type + `func(ctx, chatID, text) error` defaulting to `b.SendMessage` + wrapper. Tests inject a recorder. + +## Success Criteria + +- [ ] `go build ./...` succeeds. +- [ ] `go test ./internal/deploynotify/...` passes. +- [ ] `go test ./...` passes (no regressions). +- [ ] `cmd/server/main.go` still under reasonable size; deploynotify call + adds ≤6 LOC. +- [ ] Manual review: a `Run` invocation with empty SHA touches neither KV + nor Telegram (traceable in code, not just behaviour). + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Cold-start storm sends duplicate DMs (2+ instances boot concurrently after deploy) | Documented in plan Out-of-Scope; race window narrow; failure mode is annoyance not corruption. | +| KV write succeeds, Telegram send fails | Order is reversed: send first, write only on send success. So a failed send doesn't permanently silence retries. | +| KV read fails (DynamoDB throttle) | Treat as "not notified yet" → fall through to send. Worst case: extra DM. | +| Test for Run-with-bot leaks a goroutine | Run is synchronous (uses Timeout via context.WithTimeout). No goroutine spawned. | + +## Security Considerations + +- Owner ID is already in env; no new secret material. +- Telegram message body contains only the short git SHA — public + information (the repo is public). No env/secrets leak risk. +- KV partition `deploynotify` is read/write under existing IAM scope. diff --git a/plans/260522-1109-deploy-notify-owner/phase-02-wire-git-sha-into-build.md b/plans/260522-1109-deploy-notify-owner/phase-02-wire-git-sha-into-build.md new file mode 100644 index 0000000..4e9235b --- /dev/null +++ b/plans/260522-1109-deploy-notify-owner/phase-02-wire-git-sha-into-build.md @@ -0,0 +1,98 @@ +--- +phase: 2 +title: Wire git SHA into build +status: completed +priority: P3 +effort: 20m +dependencies: + - 1 +--- + +# Phase 2: Wire git SHA into build + +## Overview + +Inject the short git SHA into the binary at link time via +`-X main.gitSHA=…`. Without this, Phase 1's `Run` short-circuits and the +feature is dormant. Touches Makefile only; GitHub Actions already runs +`make build-lambda`, so no workflow change is required. + +## Requirements + +**Functional** +- `make build-lambda` produces a binary whose `main.gitSHA` equals + `git rev-parse --short HEAD` at build time. +- `make build` (local host binary) does the same — useful for dogfooding. +- Both targets degrade gracefully if `git` is unavailable: empty SHA → + Phase 1's `Run` silently skips. + +**Non-functional** +- No new tools or actions added to CI. +- `actions/checkout@v6` default depth (shallow) must support + `git rev-parse --short HEAD` — it does, HEAD is always present. + +## Architecture + +```makefile +GIT_SHA := $(shell git rev-parse --short HEAD 2>/dev/null) + +LDFLAGS := -s -w -X main.gitSHA=$(GIT_SHA) + +build: + CGO_ENABLED=0 go build -ldflags="$(LDFLAGS)" -o ./bin/server ./cmd/server + +build-lambda: + @mkdir -p $(dir $(LAMBDA_OUT)) + CGO_ENABLED=0 GOOS=$(LAMBDA_GOOS) GOARCH=$(LAMBDA_GOARCH) \ + go build -tags lambda.norpc -ldflags="$(LDFLAGS)" \ + -o $(LAMBDA_OUT) ./cmd/server +``` + +## Related Code Files + +- Modify: `Makefile` — add `GIT_SHA` + `LDFLAGS` vars, swap inline + `-ldflags="-s -w"` for `-ldflags="$(LDFLAGS)"` in both `build` and + `build-lambda` targets. + +## Implementation Steps + +1. **Add Makefile variables** near the top (after existing `LAMBDA_OUT` etc): + ```makefile + GIT_SHA := $(shell git rev-parse --short HEAD 2>/dev/null) + LDFLAGS := -s -w -X main.gitSHA=$(GIT_SHA) + ``` + +2. **Update `build` target** (`Makefile:51-52`): + - Change `-ldflags="-s -w"` → `-ldflags="$(LDFLAGS)"`. + +3. **Update `build-lambda` target** (`Makefile:54-60`): + - Change `-ldflags="-s -w"` → `-ldflags="$(LDFLAGS)"`. + +4. **Verify**: + - `make build` then `strings ./bin/server | grep -E '^[0-9a-f]{7,}$'` + should reveal the SHA. + - Or: add a no-op `--version`-style log line behind a build flag — + skip for now (YAGNI). + +## Success Criteria + +- [ ] `make build` builds successfully. +- [ ] `make build-lambda` builds successfully. +- [ ] Resulting binary has `main.gitSHA` populated (verified once via + `strings` grep or a temporary debug log — not a permanent test). +- [ ] No change to `.github/workflows/deploy.yml` — `make build-lambda` + in CI picks up the new ldflags automatically. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Dockerfile builds (CI `docker build -t miti99bot`) bypass Makefile and won't inject SHA | Out of scope — Lambda deploy uses `make build-lambda`, not Docker. CI's `docker build` is just a smoke check. Document the gap; revisit only if Cloud Run path is reactivated. | +| Local `make build` in a tarball download with no `.git/` → SHA empty | Acceptable — feature silently disables on non-git builds. | +| `git rev-parse` outputs spaces / unexpected chars → breaks ldflags | `git rev-parse --short HEAD` output is `[0-9a-f]{7,}` only. Safe. | +| Reproducible builds tooling complains about non-deterministic SHA | Not relevant for this project. | + +## Security Considerations + +- Short git SHA is published on every GitHub commit page — not sensitive. +- No build-time secrets touched. diff --git a/plans/260522-1109-deploy-notify-owner/plan.md b/plans/260522-1109-deploy-notify-owner/plan.md new file mode 100644 index 0000000..9061a53 --- /dev/null +++ b/plans/260522-1109-deploy-notify-owner/plan.md @@ -0,0 +1,66 @@ +--- +title: Deploy notification to bot owner with git SHA +description: >- + On startup the bot DMs BOT_OWNER_ID with the deployed git SHA. Dedup via + DynamoDB so only real new versions notify, not every Lambda cold start. +status: completed +priority: P3 +branch: main +tags: + - ops + - observability + - telegram +blockedBy: [] +blocks: [] +created: '2026-05-22T04:10:07.522Z' +createdBy: 'ck:plan' +source: skill +--- + +# Deploy notification to bot owner with git SHA + +## Overview + +Operator awareness of deploys. After `make build-lambda` + `sam deploy`, the +owner currently has no in-Telegram signal that the new code is running on +Lambda (only the GitHub Actions log). Add a startup hook that DMs the owner +with the baked-in short git SHA, deduped by KV so subsequent cold starts of +the same version stay silent. + +Confirmation that the **new code is running**, not just that the deploy +script finished — that's why this lives in the bot binary, not in the deploy +workflow. + +## Design Decisions (locked) + +- **Dedup**: DynamoDB KV stores `last_notified_sha`. Send only when baked + `gitSHA != stored`, then write. One `GetItem` per cold start (~free tier). +- **Code placement**: new `internal/deploynotify/` package — testable in + isolation, ~50 LOC, main.go just calls `deploynotify.Run(...)`. +- **Build wiring**: `-ldflags "-X main.gitSHA="` in Makefile. + Empty `gitSHA` (local non-make build) → silently skip. +- **Failure policy**: log + continue. Never block server startup. KV error, + Telegram error, missing owner — all non-fatal. +- **Timing**: synchronous, ≤3s timeout, runs after `modules.Install` and + before `srv.ListenAndServe()`. Lambda init phase has 10s headroom. + +## Phases + +| Phase | Name | Status | +|-------|------|--------| +| 1 | [Implement deploynotify package + main.go hook](./phase-01-implement-deploynotify-package-main-go-hook.md) | Completed | +| 2 | [Wire git SHA into build](./phase-02-wire-git-sha-into-build.md) | Completed | + +## Dependencies + +None. + +## Out of Scope + +- Multi-environment routing (prod vs staging) — single owner, single env today. +- Notify on rollback — same dedup mechanism naturally handles it; rollback to + a previously-notified SHA just resends because stored value moved forward. + Acceptable. +- Conditional KV write to prevent concurrent-cold-start dupes — KV interface + has no CAS today; the race window is narrow and the failure mode is + "two identical DMs", not data corruption.