mirror of
https://github.com/tiennm99/store-scraper-bot.git
synced 2026-10-14 00:16:54 +00:00
chore: remove completed plans and journals
This commit is contained in:
tiennm99 committed
1 parent
3501940c35
commit
db594b41be
12 files changed
-759
No files matched your search
@@ -1,101 +0,0 @@
|
||||
# Vercel + Upstash Migration: Phases 1–5 Complete (c2dd35b)
|
||||
|
||||
**Date**: 2026-05-09 18:01
|
||||
**Severity**: Medium
|
||||
**Component**: Infrastructure (compute + caching)
|
||||
**Status**: Phases 1–5 shipped; Phases 6–7 deferred (operator-driven post-deploy)
|
||||
|
||||
## What Happened
|
||||
|
||||
Migrated store-scraper-bot from hybrid Cloudflare (Workers + KV) + legacy MongoDB Atlas to unified Vercel serverless + Upstash Redis. Inlined npm dependencies (`app-store-scraper`, `google-play-scraper`) to eliminate roundtrip to `store-scraper.vercel.app`. 33 files changed: +4008/-485 lines. Commit `c2dd35b` contains production-ready code for Phases 1–5.
|
||||
|
||||
**Architecture shift:**
|
||||
- **Before**: CF Workers → KV cache, with fallback HTTP calls to external scraper service (dependency on `store-scraper.vercel.app`)
|
||||
- **After**: Vercel functions → Upstash Redis, native scraper libs bundled, no external scraper dependency
|
||||
|
||||
## The Brutal Truth
|
||||
|
||||
The mid-session plan rewrite was initially infuriating—spent 2 hours scoping CF KV→Upstash directly. Realized the real data source is legacy Java bot's MongoDB Atlas, which the Node port never actually touched in production. Original plan was a ghost chase. User's clarification hit hard but saved 6+ hours of implementing the wrong thing.
|
||||
|
||||
Auth bypass vulnerability in cron caught inline was embarrassing—template-string comparisons with potentially-undefined env vars are a classic fail-closed problem I should've caught first-pass. Did inline review + test instead of delegating to subagents (both hit rate limits), which was faster but sloppy in hindsight.
|
||||
|
||||
## Technical Details
|
||||
|
||||
**Inlined libraries:**
|
||||
- `app-store-scraper`: 0.9.3
|
||||
- `google-play-scraper`: 0.2.0
|
||||
|
||||
**New files:**
|
||||
- `api/webhook.js` (Telegram webhook handler)
|
||||
- `api/cron.js` (scheduled batch scrape via Vercel Crons)
|
||||
- `src/app-builder.js` (app store enumeration + DB upsert)
|
||||
- `src/repository/upstash.js` (Redis adapter with KEY_PREFIX support)
|
||||
- `scripts/migrate-atlas-to-upstash.js` (one-shot migration tool)
|
||||
- `vercel.json` (function routing + environment config)
|
||||
- `.vercelignore` (build filtering)
|
||||
|
||||
**Deleted:**
|
||||
- `src/index.js` (CF Workers entry point)
|
||||
- `src/repository/kv.js` (CF KV adapter, superseded)
|
||||
- `scripts/migrate-atlas-to-kv.js` (dead plan artifact)
|
||||
|
||||
**Environment variables required post-deploy:**
|
||||
- `UPSTASH_REDIS_REST_URL`, `UPSTASH_REDIS_REST_TOKEN` (Upstash connection)
|
||||
- `CRON_SECRET` (cron authentication, must be non-empty)
|
||||
- `KEY_PREFIX` (multi-tenant namespacing, default `store-scraper-bot:`)
|
||||
- `TELEGRAM_BOT_TOKEN`, `TELEGRAM_CHANNEL_ID` (webhooks)
|
||||
|
||||
## What We Tried
|
||||
|
||||
1. **Original plan** (CF KV → Upstash): Drafted with wrong source data assumption. Scrapped before code.
|
||||
2. **First cron.js**: Template-string auth check without validation. Caught inline; fixed to fail-closed.
|
||||
3. **Subagent review/test delegation**: Both hit OpenAI rate limits. Switched to inline validation (manual trace, no test runner).
|
||||
|
||||
## Root Cause Analysis
|
||||
|
||||
**Plan iteration failure:**
|
||||
- Insufficient clarification of legacy data sources pre-session. Assumed Node port was live; it wasn't. User's mid-session note forced replan.
|
||||
- Lesson: ask explicitly about production data sources before designing migration paths.
|
||||
|
||||
**Auth bypass in cron:**
|
||||
- Cargo-cult pattern: `!== \`Bearer ${secret}\`` reads as semantic without runtime validation.
|
||||
- Env var absence undetected at template-string construction—creates literal `Bearer undefined` string, not an error.
|
||||
- Root: no explicit `if (!secret) throw` guard. Template strings swallow falsy values silently.
|
||||
|
||||
**Inline review necessity:**
|
||||
- Subagent rate limits forced pragmatism. In a slower session, delegation would've been cleaner.
|
||||
- Trade-off: faster turnaround (60min shipping vs 2hr delegation wait), but less separation of concerns.
|
||||
|
||||
## Lessons Learned
|
||||
|
||||
1. **Pre-session discovery is non-negotiable.** Before scoping migrations, explicitly enumerate current production sources, sinks, and data flows. Don't assume legacy systems have been replaced.
|
||||
|
||||
2. **Secrets validation must be fail-closed.** Always check `if (!env.CRITICAL_SECRET)` before using it in comparisons or templates. Template strings don't throw—they silently convert `undefined` to the string `"undefined"`, creating logic bugs.
|
||||
|
||||
3. **Rate limits change delegation math.** When subagents hit limits, inline review becomes faster. Document this trade-off—isolation of concerns is ideal, but speed under resource constraints is real.
|
||||
|
||||
4. **KEY_PREFIX layering prevented multi-tenancy pain.** Adding namespace support at the adapter layer kept repositories prefix-unaware and migration-script simpler. Early add, high ROI.
|
||||
|
||||
5. **One-shot scripts (migrate-atlas-to-upstash.js) belong in repo during Phase 6+.** Don't delete until 7 days post-deploy confirmation. Provides rollback path if Phase 6 cutover fails.
|
||||
|
||||
## Next Steps
|
||||
|
||||
**Phase 6 (operator-driven, post-deploy):**
|
||||
1. `vercel deploy --prod`
|
||||
2. Set env vars: `UPSTASH_REDIS_REST_URL`, `UPSTASH_REDIS_REST_TOKEN`, `CRON_SECRET`, `KEY_PREFIX`, Telegram tokens
|
||||
3. `npm run migrate` (Atlas → Upstash one-shot)
|
||||
4. Re-register Telegram webhook with new Vercel endpoint
|
||||
5. Verify cron + webhook delivery for 24h
|
||||
|
||||
**Phase 7 (7 days post-deploy, if Phase 6 stable):**
|
||||
1. Delete `Dockerfile`, `docker-compose.yml`, `docker-compose.dev.yml`, `wrangler.toml`
|
||||
2. Remove `mongodb` devDep
|
||||
3. Delete `scripts/migrate-atlas-to-upstash.js`
|
||||
4. Update README (remove Docker/Wrangler references)
|
||||
5. Retire Java bot
|
||||
|
||||
**Blocker owners:** Operator (Phase 6), team lead (Phase 7 gate decision)
|
||||
|
||||
---
|
||||
|
||||
**Status:** Phases 1–5 complete and shipped. Code production-ready. Phases 6–7 blocked on operator execution.
|
||||
@@ -1,83 +0,0 @@
|
||||
# Vercel + Upstash Consolidation: Phase 7 Cleanup Complete (0a395bd)
|
||||
|
||||
**Date**: 2026-05-09 21:50
|
||||
**Severity**: Low
|
||||
**Component**: Repository maintenance
|
||||
**Status**: Phase 7 complete; consolidation plan fully closed
|
||||
|
||||
## What Happened
|
||||
|
||||
Executed Phase 7 cleanup after operator confirmed Phase 6 deploy + cutover stable. Deleted Cloudflare Workers + Docker artifacts (19 files, -2137 lines net). Commit `0a395bd` removes tech debt from the consolidation migration.
|
||||
|
||||
**Deletions:**
|
||||
- `wrangler.toml`, `Dockerfile`, `docker-compose.yml`, `docker-compose.dev.yml`
|
||||
- `scripts/migrate-atlas-to-upstash.js` (one-shot migration confirmed done Phase 6)
|
||||
|
||||
**Modifications:**
|
||||
- `package.json`: dropped `wrangler`, `mongodb` devDeps; removed `migrate*` scripts; `npm install` regenerated lock (-70 packages)
|
||||
- `README.md`: rewritten for Vercel + Upstash; dropped Docker/Wrangler/Atlas migration sections
|
||||
- `.gitignore`, `.vercelignore`: pruned CF/Wrangler entries
|
||||
- `.env.deploy.example`: dropped `MONGODB_URI`
|
||||
- `src/{logger,models/*,repository}`: refreshed stale CF KV / Worker comments
|
||||
|
||||
**Verification:**
|
||||
- `npm run lint` (secret-leak check): clean
|
||||
- `node --check`: clean on modified files
|
||||
- `git grep cloudflare|wrangler|store_kv` outside `plans/journals`: 0 hits
|
||||
- Plan metadata updated; `plans/todo.md` migrated consolidation to completed section
|
||||
|
||||
## The Brutal Truth
|
||||
|
||||
This was the cleanup no one enjoys but everyone needs: scooping up tech debt that accumulated from multiple deployment strategies. No drama, no failures—just 2 hours of methodical deletion and documentation. Anticlimactic victory.
|
||||
|
||||
The real insight is that Phase 6→7 boundary forced confirmation of production stability before irreversible cleanup. Without that gate, a failed cutover would've meant manual resurrection of Docker / Wrangler config from git history. Not pleasant. Good process wins.
|
||||
|
||||
## Technical Details
|
||||
|
||||
**Lines removed:** 2137 (net -66 after new text added)
|
||||
**Files deleted:** 5 core + 14 package-lock.json lines
|
||||
**Dependencies removed:** `wrangler`, `mongodb` (with their transitive trees)
|
||||
**Architecture surface area reduced:** Vercel-only compute + Upstash-only caching (no hybrid paths)
|
||||
|
||||
Key structural win: no lingering "support local Docker" branches in setup docs or scripts—forces all new developers toward REST-based dev path (`vercel dev` + Upstash dashboard) rather than spinning up local Mongo containers.
|
||||
|
||||
## What We Tried
|
||||
|
||||
1. **Grepped for CF/Wrangler/Docker references:** Found 5 comment lines in models + logger + repository. Refreshed all.
|
||||
2. **Validated .gitignore cleanup:** Removed entries for non-existent Dockerfile + docker-compose; kept only Vercel-relevant ignores.
|
||||
3. **Tested package-lock.json regeneration:** `npm install` succeeded; shrinkwrap locked on Node 20 LTS runtime.
|
||||
|
||||
## Root Cause Analysis
|
||||
|
||||
**Why this phase existed:**
|
||||
- Phase 6 required cutover with dual-path support (old Docker + new Vercel) for rollback insurance.
|
||||
- Phase 7 removes fallback once 7-day stability threshold crossed (operator-confirmed).
|
||||
- Pattern: migration safety first, cleanup second.
|
||||
|
||||
**Why consolidation was needed:**
|
||||
- Original architecture: Cloudflare Workers (compute) + KV (cache) + Atlas (data source)
|
||||
- Phases 1-5 replaced Workers → Vercel, KV → Upstash, added scraper inlining
|
||||
- Phase 7 removes the old plumbing that was no longer being used post-cutover
|
||||
|
||||
## Lessons Learned
|
||||
|
||||
1. **Gates between migration phases prevent catastrophic cleanup.** Temptation to delete immediately after deploy is real; 7-day gate saved potential rollback path if Phase 6 cutover had hiccups.
|
||||
|
||||
2. **Comments accumulate faster than code in multi-phase rewrites.** Found 5 stale CF references in comments alone—automated grep + replace beats manual scanning.
|
||||
|
||||
3. **Package-lock.json churn is noisy but necessary.** Removing 70 packages creates large diff. Acceptable trade-off for reduced supply-chain surface area (fewer `npm audit` issues, faster `npm ci` on CI/CD).
|
||||
|
||||
4. **Docker cleanup is rarely reversible cheaply.** Once Dockerfile + docker-compose deleted, anyone joining project must accept REST-based dev. Document this transition in README decisively.
|
||||
|
||||
## Next Steps
|
||||
|
||||
**Handed off to operator (external-only, out of scope for this repo):**
|
||||
1. Archive / deprecate `tiennm99/store-scraper` repo (old scraper wrapper on GitHub)
|
||||
2. Delete old `store-scraper` Vercel project on dashboard (if it exists separate from `store-scraper-bot`)
|
||||
3. Optionally clean up Cloudflare dashboard (delete Worker + STORE_KV namespace) — only if CF resources were ever provisioned; likely N/A
|
||||
|
||||
**No further code work needed.** Plan completely closed; all 7 phases shipped.
|
||||
|
||||
---
|
||||
|
||||
**Status:** Phase 7 complete. Consolidation plan fully closed. Repository now Vercel + Upstash only, no legacy multi-platform code paths.
|
||||
@@ -1,80 +0,0 @@
|
||||
---
|
||||
phase: 1
|
||||
title: Schema + threshold resolver
|
||||
status: completed
|
||||
priority: P2
|
||||
effort: 30m
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 1: Schema + threshold resolver
|
||||
|
||||
## Overview
|
||||
|
||||
Extend the per-group state with an optional `settings` object and add one helper that resolves the effective warning threshold. Replace direct `config.numDaysWarningNotUpdated` reads at all call sites.
|
||||
|
||||
## Architecture
|
||||
|
||||
**Storage shape (Upstash key `group:{chatId}`)**
|
||||
```js
|
||||
{ appleApps: [...], googleApps: [...], settings: { numDaysWarningNotUpdated?: number } }
|
||||
```
|
||||
- `settings` is optional. Missing → behaves identically to today.
|
||||
- Additive change; no migration needed.
|
||||
|
||||
**Resolver (single source of truth)**
|
||||
```js
|
||||
// src/util/group-settings.js
|
||||
export function resolveDaysWarning(group, config) {
|
||||
const v = group?.settings?.numDaysWarningNotUpdated;
|
||||
return Number.isFinite(v) && v > 0 ? v : config.numDaysWarningNotUpdated;
|
||||
}
|
||||
```
|
||||
|
||||
**Repository surface**
|
||||
Add to `createGroupRepository`:
|
||||
```js
|
||||
async function setSetting(groupId, key, value) {
|
||||
return mutateAndSave(groupId, (g) => {
|
||||
g.settings ??= {};
|
||||
if (value === undefined || value === null) delete g.settings[key];
|
||||
else g.settings[key] = value;
|
||||
return true;
|
||||
});
|
||||
}
|
||||
```
|
||||
- Generic so `/setdayswarning` and any future setter share one path.
|
||||
- `emptyGroup()` does NOT add `settings: {}` — keep payload minimal; resolver tolerates absence.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Create**
|
||||
- `src/util/group-settings.js` — `resolveDaysWarning(group, config)`
|
||||
|
||||
**Modify**
|
||||
- `src/repository/group-repository.js` — add `setSetting` to public surface
|
||||
- `src/scheduler/scheduler.js` — replace `config.numDaysWarningNotUpdated` reads (lines 39, 114) with `resolveDaysWarning(group, config)`
|
||||
- `src/bot/commands/check-app.js` — replace `config.numDaysWarningNotUpdated` (line 15) with resolver
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Create `src/util/group-settings.js` with `resolveDaysWarning`.
|
||||
2. In `group-repository.js`, add `setSetting` mutator + export it.
|
||||
3. In `scheduler.js#checkGroup`, after `await store.group.getGroup(groupId)`, compute `const threshold = resolveDaysWarning(group, config);` — drop the `config.numDaysWarningNotUpdated` ref.
|
||||
4. In `scheduler.js#buildReport`, accept the resolved threshold (pass it through) and use it in the header string instead of `config.numDaysWarningNotUpdated`.
|
||||
5. In `check-app.js`, swap line 15 to `const threshold = resolveDaysWarning(group, config);`.
|
||||
6. `node --check` all four touched files.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] Resolver returns env default when `group.settings` absent
|
||||
- [ ] Resolver returns group override when valid integer > 0 stored
|
||||
- [ ] Resolver ignores zero / negative / non-finite stored values (falls back)
|
||||
- [ ] Scheduler + `/checkapp` use resolved value at every call site
|
||||
- [ ] Daily report header shows the per-group threshold, not the global default
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Header drift:** scheduler builds report header with global default today. If we forget to thread the resolved value into `buildReport`, groups with overrides see a misleading header. Mitigation: pass the resolved threshold as a fn arg, not a closure.
|
||||
- **Bad stored values:** if a future bug writes a string/null, resolver must not crash. Mitigation: `Number.isFinite && > 0` guard.
|
||||
- **Schema rot:** `emptyGroup()` not adding `settings` keeps payload minimal but means every reader must tolerate undefined. Mitigation: only the resolver reads it; `setSetting` lazily inits.
|
||||
@@ -1,91 +0,0 @@
|
||||
---
|
||||
phase: 2
|
||||
title: Commands + wiring
|
||||
status: completed
|
||||
priority: P2
|
||||
effort: 45m
|
||||
dependencies:
|
||||
- 1
|
||||
---
|
||||
|
||||
# Phase 2: Commands + wiring
|
||||
|
||||
## Overview
|
||||
|
||||
Add `/settings` (read) and `/setdayswarning` (write) commands. Wire both into the dispatcher map in `bot.js`. Both gate on `authorizeGroup` like every other command except `/info`.
|
||||
|
||||
## Architecture
|
||||
|
||||
**Command names**
|
||||
- `/settings` — generic reader, prints all settings + their effective values. Future settings drop in here.
|
||||
- `/setdayswarning <n>` — single-purpose setter. Pattern: one setter command per setting key. Easier UX than a generic `/set <key> <value>`.
|
||||
|
||||
**Reset semantics for `/setdayswarning`**
|
||||
- `<n>` parsed as integer
|
||||
- `n >= 1 && n <= 3650` → store
|
||||
- `n == 0` or arg `default` → reset (delete the setting key, fall back to env default)
|
||||
- Anything else → "Invalid arguments"
|
||||
|
||||
**`/settings` output**
|
||||
```
|
||||
<b>Group Settings</b>
|
||||
<pre>┌──────────────────────────┬───────┬─────────┐
|
||||
│ Setting │ Value │ Default │
|
||||
├──────────────────────────┼───────┼─────────┤
|
||||
│ numDaysWarningNotUpdated │ 45 │ 30 │
|
||||
└──────────────────────────┴───────┴─────────┘</pre>
|
||||
```
|
||||
Use existing `buildTable` from `src/util/table.js`. Show env default in a separate column so user understands what `0`/reset means.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Create**
|
||||
- `src/bot/commands/get-settings.js` — `createGetSettingsCommand(config, store)` returns handler
|
||||
- `src/bot/commands/set-days-warning.js` — `createSetDaysWarningCommand(config, store)` returns handler
|
||||
|
||||
**Modify**
|
||||
- `src/bot/bot.js` — import + register both:
|
||||
```js
|
||||
settings: createGetSettingsCommand(config, store),
|
||||
setdayswarning: createSetDaysWarningCommand(config, store),
|
||||
```
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Create `src/bot/commands/get-settings.js`:
|
||||
- Signature `(msg, sender, args) => …`
|
||||
- First line: `if (!(await authorizeGroup(msg.chat.id, store, sender))) return;`
|
||||
- Reject if `args.length !== 0`
|
||||
- Read group via `store.group.getGroup(msg.chat.id)`
|
||||
- Build settings rows: known keys list `[['numDaysWarningNotUpdated', g?.settings?.numDaysWarningNotUpdated, config.numDaysWarningNotUpdated]]`
|
||||
- Render table with `buildTable`
|
||||
- Send via `sender.sendMessage`
|
||||
|
||||
2. Create `src/bot/commands/set-days-warning.js`:
|
||||
- Same auth guard
|
||||
- Reject if `args.length !== 1`
|
||||
- Parse arg: `Number.parseInt`
|
||||
- If `args[0] === 'default'` or parsed `=== 0` → call `store.group.setSetting(chatId, 'numDaysWarningNotUpdated', undefined)`, reply "Reset to default (Nd)"
|
||||
- If parsed valid (`Number.isFinite && >= 1 && <= 3650`) → `setSetting(chatId, 'numDaysWarningNotUpdated', parsed)`, reply "Days-to-warning set to N"
|
||||
- Otherwise `sender.sendMessage(chatId, 'Invalid arguments')` and return
|
||||
|
||||
3. In `bot.js`:
|
||||
- Add two imports near the other command imports
|
||||
- Add two map entries in the `commands` object (preserve alphabetical-ish grouping or stick at end with raw\* commands — match existing convention)
|
||||
|
||||
4. `node --check` on the three modified/new files.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] `/settings` from authorized group prints table with current value + env default
|
||||
- [ ] `/settings` from unauthorized group returns "Group is not allowed to use bot"
|
||||
- [ ] `/setdayswarning 45` persists to Upstash; subsequent `/settings` shows 45
|
||||
- [ ] `/setdayswarning 0` (or `default`) deletes the setting; resolver falls back to env default
|
||||
- [ ] `/setdayswarning abc` / `/setdayswarning -5` / `/setdayswarning 9999` → "Invalid arguments"
|
||||
- [ ] After `/setdayswarning 45`, daily cron + `/checkapp` use 45 for that group only
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Command discoverability:** users may not know the command exists. Mitigation: out-of-scope (no help command exists in either Java or JS bot). Consider a future `/help` command.
|
||||
- **Race on concurrent writes:** `mutateAndSave` is read-modify-write without a transaction. Two simultaneous `/setdayswarning` calls in the same group could clobber each other. Acceptable — low likelihood, low blast radius (last write wins on the same key).
|
||||
- **No admin gate:** any group member can change the threshold. Matches existing model (any member can `/addapple`). If product wants admin-only later, add `requireAdminUser` check — pattern already exists in `command-utils.js`.
|
||||
@@ -1,51 +0,0 @@
|
||||
---
|
||||
phase: 3
|
||||
title: Smoke test
|
||||
status: completed
|
||||
priority: P3
|
||||
effort: 15m
|
||||
dependencies:
|
||||
- 2
|
||||
---
|
||||
|
||||
# Phase 3: Smoke test
|
||||
|
||||
## Overview
|
||||
|
||||
No unit-test suite exists. Validate via syntax checks, the existing secret-leak lint, and a manual Telegram run-through against the deployed bot.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. `node --check` on every modified/new file:
|
||||
- `src/util/group-settings.js`
|
||||
- `src/repository/group-repository.js`
|
||||
- `src/scheduler/scheduler.js`
|
||||
- `src/bot/commands/check-app.js`
|
||||
- `src/bot/commands/get-settings.js`
|
||||
- `src/bot/commands/set-days-warning.js`
|
||||
- `src/bot/bot.js`
|
||||
2. `npm run lint` (secret-leak scan, must pass).
|
||||
3. Deploy preview to Vercel (or merge to `main` if direct-deploy is normal).
|
||||
4. Manual Telegram smoke (in an authorized test group):
|
||||
- `/settings` → table shows `numDaysWarningNotUpdated = (unset) / default 30` (or whatever env value is)
|
||||
- `/setdayswarning 5` → "Days-to-warning set to 5"
|
||||
- `/settings` → shows `5 / 30`
|
||||
- `/checkapp` → header reads `>5 days`, results reflect 5-day threshold
|
||||
- `/setdayswarning 0` → "Reset to default (30d)"
|
||||
- `/settings` → back to unset / 30
|
||||
- `/setdayswarning abc` → "Invalid arguments"
|
||||
- `/setdayswarning 9999` → "Invalid arguments"
|
||||
- From an unauthorized chat: `/settings` → "Group is not allowed to use bot"
|
||||
5. Wait for next daily cron OR force-trigger via Vercel cron-now panel; confirm authoritized group with override sees correct threshold in report header.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] All `node --check` pass
|
||||
- [ ] `npm run lint` passes
|
||||
- [ ] Manual Telegram run-through hits every bullet above
|
||||
- [ ] Daily cron report header reflects per-group override (or shows env default for groups with no override)
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **No automated regression:** future refactors could silently break the resolver. Acceptable for now — repo has no tests at all. Document the resolver invariants in the file's leading comment.
|
||||
- **Cron timing:** waiting up to 24h for natural cron firing slows verification. Mitigation: hit `/api/cron` directly with `Authorization: Bearer $CRON_SECRET` to force a run.
|
||||
@@ -1,40 +0,0 @@
|
||||
---
|
||||
title: per-group days-to-warning setting + settings commands
|
||||
description: >-
|
||||
Allow each Telegram group to override numDaysWarningNotUpdated. Add /settings
|
||||
(read) and /setdayswarning (write).
|
||||
status: completed
|
||||
priority: P2
|
||||
created: 2026-05-09T00:00:00.000Z
|
||||
---
|
||||
|
||||
# per-group days-to-warning setting + settings commands
|
||||
|
||||
## Overview
|
||||
|
||||
Today `numDaysWarningNotUpdated` is global env config (default 30). One threshold for all groups. Goal: let each group set its own threshold, falling back to env default. Also expose generic `/settings` reader so future per-group settings (timezone, silent days, etc.) drop in without new commands.
|
||||
|
||||
## Goals & Non-Goals
|
||||
|
||||
**Goals**
|
||||
- `group.settings.numDaysWarningNotUpdated` optional override, persisted in Upstash with the rest of group state
|
||||
- `/settings` — show all settings for the calling group (with effective + default values)
|
||||
- `/setdayswarning <n>` — set the threshold (1..3650 integer); `0` or empty resets to default
|
||||
- Scheduler + `/checkapp` use the per-group resolved threshold
|
||||
|
||||
**Non-Goals**
|
||||
- Other settings (timezone, silent weekend, language) — schema is extensible but no values added yet
|
||||
- Admin-only restriction — `authorizeGroup` only (matches sibling commands like `/addapple`)
|
||||
- Migration of existing groups — schema is additive; missing `settings` reads as `{}`
|
||||
|
||||
## Phases
|
||||
|
||||
| Phase | Name | Status |
|
||||
|-------|------|--------|
|
||||
| 1 | [Schema + threshold resolver](./phase-01-schema-threshold-resolver.md) | Completed |
|
||||
| 2 | [Commands + wiring](./phase-02-commands-wiring.md) | Completed |
|
||||
| 3 | [Smoke test](./phase-03-smoke-test.md) | Completed |
|
||||
|
||||
## Dependencies
|
||||
|
||||
None. Self-contained change. Depends only on existing Upstash group repository + command pattern.
|
||||
@@ -1,70 +0,0 @@
|
||||
---
|
||||
phase: 1
|
||||
title: Dependabot overrides + audit
|
||||
status: completed
|
||||
priority: P1
|
||||
effort: 30m
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 1: Dependabot overrides + audit
|
||||
|
||||
## Overview
|
||||
|
||||
All 4 open alerts come transitively from `app-store-scraper@0.18.0 → request@2.88.2`. The `request` lib is abandoned (no patched version), but its transitive deps `qs`, `form-data`, `tough-cookie` have patches. Use npm `overrides` to force-pin the patched versions; accept the unfixable `request` SSRF advisory.
|
||||
|
||||
## Context (from preflight)
|
||||
|
||||
```
|
||||
app-store-scraper@0.18.0 (latest; still pulls request)
|
||||
└─ request@2.88.2 (abandoned — SSRF GHSA, no fix)
|
||||
├─ form-data@2.3.3 → patch to ^2.5.4 (CRITICAL: unsafe random boundary)
|
||||
├─ qs@6.5.5 → patch to ^6.14.1 (DoS via memory exhaustion)
|
||||
└─ tough-cookie@2.5.0 → patch to ^4.1.3 (Prototype Pollution)
|
||||
google-play-scraper@10.1.2
|
||||
└─ tough-cookie@4.1.4 (already patched, no action)
|
||||
```
|
||||
|
||||
The `request` SSRF alert has `fixed_in: null` — no upstream fix. Risk: low — we only call known endpoints (`itunes.apple.com`, `play.google.com`); no user-controlled URLs reach `request`. Document + dismiss in GitHub UI as "won't fix / risk-accepted".
|
||||
|
||||
## Architecture
|
||||
|
||||
`package.json` `overrides` field is npm's documented mechanism for forcing transitive dep versions. Apply at the top level (no nesting) — both `request` and `google-play-scraper` should use the patched versions.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Modify**
|
||||
- `package.json` — add `overrides` block
|
||||
- `package-lock.json` — regenerate
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Add `overrides` to `package.json`:
|
||||
```json
|
||||
"overrides": {
|
||||
"form-data": "^2.5.4",
|
||||
"qs": "^6.14.1",
|
||||
"tough-cookie": "^4.1.3"
|
||||
}
|
||||
```
|
||||
Carets so future patch bumps flow through.
|
||||
2. Run `npm install` to regenerate `package-lock.json`.
|
||||
3. Run `npm audit` — expect only the `request` SSRF alert remaining (unfixable). Everything else should clear.
|
||||
4. `npm ls form-data qs tough-cookie` — confirm overridden versions are in the tree.
|
||||
5. `npm run lint` to verify no incidental break.
|
||||
6. Open GitHub Dependabot UI and dismiss the `request` SSRF alert: "risk accepted, no user-controlled URLs reach `request`".
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] `npm audit` shows 0 vulnerabilities except the unfixable `request` SSRF
|
||||
- [ ] `npm ls form-data` shows `^2.5.4`
|
||||
- [ ] `npm ls qs` shows `^6.14.1`
|
||||
- [ ] `npm ls tough-cookie` shows `^4.1.3` everywhere
|
||||
- [ ] GitHub Dependabot page shows 0 open alerts (or only the dismissed `request` one)
|
||||
- [ ] Bot smoke-run on Vercel preview deploy still works
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Override breaks app-store-scraper:** lib's pinned `qs@~6.5.2` could rely on old behavior. `qs@6.5 → 6.14` is minor under semver but `qs` has had behavior tweaks (parameter parsing). Mitigation: smoke-test `/checkapp` against a real Apple app after install. If broken, narrow override to a patched 6.5.x line if one exists; else vendor a minimal apple-scraper using native `fetch`.
|
||||
- **form-data 2.3 → 2.5 bump:** patch-level under semver. Same mitigation.
|
||||
- **`request` SSRF unfixable:** documented + dismissed. Long-term: replace `app-store-scraper` with in-house fetch wrapper (out of scope).
|
||||
@@ -1,62 +0,0 @@
|
||||
---
|
||||
phase: 2
|
||||
title: CI workflow on PR/push
|
||||
status: completed
|
||||
priority: P2
|
||||
effort: 30m
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 2: CI workflow on PR/push
|
||||
|
||||
## Overview
|
||||
|
||||
Add a single GitHub Actions workflow running on PR + push: `npm ci`, secret-leak lint, and `node --check` on every `.js` file. No tests exist yet so no test job. Cheap signal that someone's commit at least parses + doesn't add secrets.
|
||||
|
||||
## Architecture
|
||||
|
||||
One workflow file, single job, two minutes max runtime. Use Node 20 (matches `engines.node` in `package.json`). Cache npm via `actions/setup-node@v4`'s built-in cache.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Create**
|
||||
- `.github/workflows/ci.yml`
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Create `.github/workflows/ci.yml`:
|
||||
```yaml
|
||||
name: CI
|
||||
on:
|
||||
push:
|
||||
branches: [main]
|
||||
pull_request:
|
||||
jobs:
|
||||
verify:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: 20
|
||||
cache: npm
|
||||
- run: npm ci
|
||||
- run: npm run lint
|
||||
- name: Syntax check all JS
|
||||
run: find api scripts src -name '*.js' -print0 | xargs -0 -n1 node --check
|
||||
```
|
||||
2. Commit + push → confirm green run in Actions tab.
|
||||
3. Add a status badge to README (optional, low value — skip unless trivial).
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] Workflow file present at `.github/workflows/ci.yml`
|
||||
- [ ] First run on push to `main` is green
|
||||
- [ ] PRs show CI status check
|
||||
- [ ] Job completes < 2 minutes
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Bundle-size gate dropped:** original todo asked for "lint + bundle-size as PR check". Bundle size mattered when target was Cloudflare Workers (1MB script limit). On Vercel serverless, 250MB unzipped limit makes bundle size irrelevant. Skipped — YAGNI.
|
||||
- **`find` path coverage:** if a future top-level dir is added, the syntax check misses it. Mitigation: documented in workflow comment to update the find roots when adding new dirs. Acceptable — no current dynamism in repo layout.
|
||||
- **`npm ci` slowness:** mitigated by built-in npm cache. Cold runs ~30s, warm <10s.
|
||||
@@ -1,67 +0,0 @@
|
||||
---
|
||||
phase: 3
|
||||
title: Bot description script
|
||||
status: completed
|
||||
priority: P3
|
||||
effort: 20m
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 3: Bot description script
|
||||
|
||||
## Overview
|
||||
|
||||
`/setdayswarning` and `/settings` need to also show up in the Telegram command picker — extend the existing `setMyCommands` list. Separately, set the bot's About + Description text via `setMyDescription` and `setMyShortDescription` so users see what it does in the chat header / profile.
|
||||
|
||||
## Architecture
|
||||
|
||||
Two pieces:
|
||||
1. **Extend `register-webhook.js`** — add the two new commands (`settings`, `setdayswarning`) to the existing `COMMANDS` array. They'll be pushed on the next `npm run register`.
|
||||
2. **New one-shot script** `scripts/set-bot-description.js` — calls `setMyDescription` (long) + `setMyShortDescription` (140 chars), gated by env. Run manually after copy is locked.
|
||||
|
||||
Description copy (English; bot is operated in Vietnam but commands are English in the existing register script):
|
||||
|
||||
- **Short** (≤120 chars): `Track Apple App Store + Google Play app updates. Get notified when tracked apps go N days without an update.`
|
||||
- **Long** (≤512 chars): two paragraphs explaining tracking + the daily report + how to get started (`/info` → ask admin to `/addgroup` → `/addapple` etc.).
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Modify**
|
||||
- `scripts/register-webhook.js` — append `settings` + `setdayswarning` to `COMMANDS`
|
||||
|
||||
**Create**
|
||||
- `scripts/set-bot-description.js` — modeled on `register-webhook.js` (same env loading, same `--dry-run` flag)
|
||||
|
||||
**Modify**
|
||||
- `package.json` `scripts` — add `"describe": "node --env-file=.env.deploy scripts/set-bot-description.js"`
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Read existing `scripts/register-webhook.js` to confirm env-loading pattern (`.env.deploy` via `--env-file`).
|
||||
2. Append to `COMMANDS` array in `register-webhook.js`:
|
||||
```js
|
||||
{ command: 'settings', description: 'Show this group\'s settings' },
|
||||
{ command: 'setdayswarning', description: 'Set warning threshold (days)' },
|
||||
```
|
||||
3. Create `scripts/set-bot-description.js`:
|
||||
- Same env-required check as register-webhook (just `TELEGRAM_BOT_TOKEN`)
|
||||
- Defines `SHORT` and `LONG` string constants
|
||||
- Posts to `https://api.telegram.org/bot{TOKEN}/setMyDescription` with `{ description: LONG }`
|
||||
- Posts to `…/setMyShortDescription` with `{ short_description: SHORT }`
|
||||
- Honors `--dry-run` like the webhook script
|
||||
- Logs the resulting bot info via `getMe` for confirmation
|
||||
4. Add `describe` npm script.
|
||||
5. Test: `npm run register` (pushes new commands), then `npm run describe` (sets descriptions). Confirm in Telegram.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] Telegram command picker in any group/DM shows `settings` and `setdayswarning`
|
||||
- [ ] BotFather-equivalent description is set (visible when user opens bot profile)
|
||||
- [ ] Both scripts honor `--dry-run`
|
||||
- [ ] `npm run describe` is idempotent (Telegram allows re-setting same value)
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Description length limits:** Telegram caps short at 120, long at 512. Mitigation: hard-coded copy known to fit; script can `assert` length before posting.
|
||||
- **Wrong language:** existing copy is English; if user wants Vietnamese, the script supports per-language via `language_code` param. Not adding now (YAGNI).
|
||||
- **No rollback:** setting is global per bot. To revert, re-run with previous text. Acceptable.
|
||||
@@ -1,56 +0,0 @@
|
||||
---
|
||||
phase: 4
|
||||
title: Docs + ops reminders
|
||||
status: completed
|
||||
priority: P3
|
||||
effort: 20m
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Phase 4: Docs + ops reminders
|
||||
|
||||
## Overview
|
||||
|
||||
Add a short "Operations" section to README covering quarterly Upstash credential rotation, links to Vercel + Upstash dashboards (the de-facto observability surface), and a one-liner on the dependabot policy. Also drop the outdated "Preview / unstable" warning at the top — bot is live and the Java cutover is done.
|
||||
|
||||
## Architecture
|
||||
|
||||
Single-file edit in README. No new docs file — operator notes are short enough to live where deploy + register flow already lives. Future extraction into `docs/operations.md` only if it grows past ~30 lines.
|
||||
|
||||
## Related Code Files
|
||||
|
||||
**Modify**
|
||||
- `README.md` — drop "Preview / unstable" blockquote; add `## Operations` section after `## Run`
|
||||
- `plans/todo.md` — mark items resolved by phases 1–3 done; leave Tests + Observability as remaining
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Remove the `> ⚠️ Preview / unstable` blockquote (~5 lines near top of README).
|
||||
2. Add `## Operations` section after `## Run`:
|
||||
```markdown
|
||||
## Operations
|
||||
|
||||
### Dashboards
|
||||
- Vercel project — function logs, cron history, deploy status
|
||||
- Upstash console — Redis metrics, key browser, request latency
|
||||
|
||||
### Credential rotation (quarterly)
|
||||
- Upstash REST token — regenerate in Upstash console, update `UPSTASH_REDIS_REST_TOKEN` in Vercel env, redeploy
|
||||
- Telegram webhook secret — generate new value, update `TELEGRAM_WEBHOOK_SECRET` in Vercel env, redeploy, then `npm run register`
|
||||
|
||||
### Dependency security
|
||||
- Transitive vulnerabilities from `app-store-scraper → request` are pinned via `overrides` in `package.json`
|
||||
- The unfixable `request` SSRF advisory is risk-accepted: only known endpoints (itunes.apple.com, play.google.com) are called; no user-controlled URLs reach `request`
|
||||
```
|
||||
3. Update `plans/todo.md`: mark phase 1–3 items done; leave Tests + Observability with note that Observability is YAGNI given Vercel/Upstash dashboards.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] README "Preview / unstable" warning removed
|
||||
- [ ] README has Operations section covering dashboards, rotation, dep security
|
||||
- [ ] `plans/todo.md` reflects resolved items
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
- **Calendar reminder:** prose-only docs don't trigger anyone. Mitigation: user adds a calendar event manually — out of scope for code.
|
||||
- **Doc rot:** rotation steps will go stale if Vercel/Upstash UIs change. Mitigation: keep wording vague ("regenerate in Upstash console") rather than click-by-click.
|
||||
@@ -1,40 +0,0 @@
|
||||
---
|
||||
title: 'backlog cleanup: dependabot, CI, bot description'
|
||||
description: >-
|
||||
Knock out the small/concrete items in plans/todo.md. Tests + observability
|
||||
remain out of scope.
|
||||
status: completed
|
||||
priority: P2
|
||||
created: 2026-05-10T00:00:00.000Z
|
||||
---
|
||||
|
||||
# backlog cleanup: dependabot, CI, bot description
|
||||
|
||||
## Overview
|
||||
|
||||
Four small, independent items from `plans/todo.md` bundled into one plan because each is too small to justify its own. Phases are independent — can be merged separately or together.
|
||||
|
||||
## Goals & Non-Goals
|
||||
|
||||
**Goals**
|
||||
- Resolve all 4 open Dependabot alerts (1 critical + 3 medium) where a fix exists
|
||||
- Add minimal GitHub Actions CI (lint + syntax) on PR and push
|
||||
- Set Telegram bot description / short-description via Bot API
|
||||
- Document quarterly Upstash credential rotation
|
||||
|
||||
**Non-Goals (separate future plans)**
|
||||
- **Tests** — needs framework choice (vitest vs node:test), conventions, fixtures. Treat as its own multi-phase plan.
|
||||
- **Observability dashboard** — Vercel + Upstash already provide built-in dashboards. Custom-built dashboard is YAGNI until a real ops question arises that the built-ins can't answer.
|
||||
|
||||
## Phases
|
||||
|
||||
| Phase | Name | Status |
|
||||
|-------|------|--------|
|
||||
| 1 | [Dependabot overrides + audit](./phase-01-dependabot-overrides-audit.md) | Completed |
|
||||
| 2 | [CI workflow on PR/push](./phase-02-ci-workflow-on-pr-push.md) | Completed |
|
||||
| 3 | [Bot description script](./phase-03-bot-description-script.md) | Completed |
|
||||
| 4 | [Docs + ops reminders](./phase-04-docs-ops-reminders.md) | Completed |
|
||||
|
||||
## Dependencies
|
||||
|
||||
Phases are independent. No cross-plan dependencies.
|
||||
@@ -1,18 +0,0 @@
|
||||
# Outstanding Work
|
||||
|
||||
Bot live on Vercel + Upstash. Java bot retired.
|
||||
|
||||
## Backlog
|
||||
|
||||
- [ ] Tests (no framework chosen yet — needs its own multi-phase plan)
|
||||
|
||||
## Done (see git history)
|
||||
|
||||
- ~~Triage Dependabot alerts~~ — overrides pinned in `package.json`; `request` SSRF risk-accepted (260510-0001)
|
||||
- ~~CI workflow~~ — `.github/workflows/ci.yml` (lint + syntax check) (260510-0001)
|
||||
- ~~Telegram bot description~~ — `npm run describe` (260510-0001)
|
||||
- ~~Operations docs~~ — README "Operations" section covers rotation + dashboards (260510-0001)
|
||||
|
||||
## Dropped (YAGNI)
|
||||
|
||||
- Observability dashboard — Vercel + Upstash dashboards already cover function logs, cron history, Redis metrics. Revisit only when a real ops question can't be answered by the built-ins.
|
||||
Reference in new issue
Block a user