From 81cde36405eb4b2dd347339c05540aab794b5c0e Mon Sep 17 00:00:00 2001 From: Juha Kangas <42040080+valuecodes@users.noreply.github.com> Date: Sun, 26 Apr 2026 10:54:35 +0300 Subject: [PATCH 1/3] chore: add /review-plan slash command --- .claude/commands/review-plan.md | 84 +++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 .claude/commands/review-plan.md diff --git a/.claude/commands/review-plan.md b/.claude/commands/review-plan.md new file mode 100644 index 0000000..d1af14c --- /dev/null +++ b/.claude/commands/review-plan.md @@ -0,0 +1,84 @@ +--- +description: Codex-powered thorough review of a plan or design document — finds holes, hidden assumptions, and bugs in the plan itself +argument-hint: "[--wait|--background] [path/to/plan.md]" +allowed-tools: Read, Write, Bash(node:*), Bash(ls:*), AskUserQuestion +--- + +Run a thorough adversarial Codex review of a plan or design document. The plan has NOT been implemented yet — Codex's job is to stress-test the plan itself, not produce code. + +Raw slash-command arguments: +`$ARGUMENTS` + +Core constraint: + +- This command is review-only. +- Do not fix issues, apply patches, or suggest you are about to make changes. +- Your only job is to run Codex and return its output verbatim. + +Locating the plan: + +1. If the arguments include a path that looks like a file (e.g. `docs/plan.md`, `./plan.md`, an absolute path), resolve it relative to the current working directory and use that file as the review target. Verify it exists with `Read` before invoking Codex. +2. If no path is given, take the most recent plan you (Claude) produced in this conversation, write its full markdown to `/tmp/review-plan-.md` with the `Write` tool, and use that file as the review target. Do not summarize or trim — write it verbatim. +3. If you cannot locate a plan in either place, ask the user where the plan lives. Do not proceed without a target. + +Execution mode rules: + +- If the arguments include `--wait`, run in the foreground without asking. +- If the arguments include `--background`, run as a background Bash task without asking. +- Otherwise, recommend foreground (`--wait`) — plan reviews are usually small and fast. Use `AskUserQuestion` exactly once with two options, recommended first: + - `Wait for results (Recommended)` + - `Run in background` + +Resolving the Codex companion: + +- The codex plugin lives under `~/.claude/plugins/cache/openai-codex/codex//scripts/codex-companion.mjs`. Resolve the latest version dynamically: + +```bash +COMPANION="$(ls -1d "$HOME"/.claude/plugins/cache/openai-codex/codex/*/scripts/codex-companion.mjs 2>/dev/null | sort -V | tail -1)" +``` + +- If `$COMPANION` is empty, tell the user to run `/codex:setup` and stop. + +Building the prompt: + +- Use the heredoc below verbatim, substituting `` with the absolute path to the plan file. +- Do NOT pass `--write`. This is read-only review — Codex must not modify any files. +- Do NOT pass `--resume-last`. Each plan review is a fresh Codex thread. + +Foreground flow: + +```bash +node "$COMPANION" task "$(cat <<'EOF' +You are reviewing a plan / design document. The plan is for a coding change that has NOT been implemented yet — your job is to stress-test the plan itself, not produce code or modify files. + +Read the file at first. Then review the plan adversarially for: + +- Hidden assumptions and where they break under real conditions. +- Missing steps, gaps, or hand-waving in the design. +- Ordering / dependency issues (e.g. migration vs deploy ordering, type changes that break callers, schema changes that aren't backward-compatible). +- Race conditions and concurrency bugs implied by the proposed runtime behavior. +- Security gaps (auth, input validation, replay, token spoofing, IDOR, allowlist bypass). +- Backwards-compatibility, rollback, and data-migration risks. +- Edge cases and failure modes the plan does not address (timeouts, partial failures, retries, idempotency). +- Test coverage gaps — both what's listed and what's silently missing. +- Vague language that hides ambiguity ("handle gracefully", "as needed", "if necessary", "etc"). +- Scope or effort that looks unrealistic for the risk implied. +- External-system contracts (APIs, webhooks) the plan relies on without verifying. + +Rules: +- If the plan is sound, say so explicitly and stop. Do not invent issues to look thorough. +- If you find issues, list them ordered by severity (Critical / High / Medium / Low). For each issue: cite the section of the plan it relates to, describe the concrete failure mode, and give a one-line fix or open question. +- End with a short verdict: "Ship as-is", "Ship after fixing the Critical/High items", or "Re-plan". +- Do not write code. Do not modify any files. Output one review only. +EOF +)" +``` + +Background flow: + +- Same command, launched via `Bash` with `run_in_background: true`. After launching, tell the user: "Codex plan review started in the background. Check `/codex:status` for progress." + +Output rules: + +- Foreground: return the stdout of `codex-companion` verbatim — no commentary before or after, no paraphrasing. +- Do not fix any issues Codex raises in the same turn. The user will follow up if they want changes. From 1c7d0082abd530b932c400f906f34353e7405b7a Mon Sep 17 00:00:00 2001 From: Juha Kangas <42040080+valuecodes@users.noreply.github.com> Date: Sun, 26 Apr 2026 11:13:21 +0300 Subject: [PATCH 2/3] chore: add pnpm-update slash command and release-age guard --- .claude/commands/pnpm-update.md | 110 ++++++++++++++++++++++++++++++++ pnpm-workspace.yaml | 2 + 2 files changed, 112 insertions(+) create mode 100644 .claude/commands/pnpm-update.md diff --git a/.claude/commands/pnpm-update.md b/.claude/commands/pnpm-update.md new file mode 100644 index 0000000..361d284 --- /dev/null +++ b/.claude/commands/pnpm-update.md @@ -0,0 +1,110 @@ +--- +description: Triage outdated pnpm packages, research what each upgrade entails, then update the approved set +allowed-tools: Bash(pnpm outdated:*), Bash(pnpm up:*), Bash(pnpm typecheck:*), Bash(pnpm lint:*), Bash(pnpm test:*), Bash(pnpm build:*), Bash(git status:*), Bash(git diff:*), Bash(cat:*), Bash(jq:*), Read, WebSearch, WebFetch +argument-hint: [--dev | --prod] +--- + +Survey outdated packages in this pnpm workspace, research what each update is, propose a targeted upgrade plan, get my confirmation, then run the update. Do NOT commit. + +Arguments: `$ARGUMENTS` — optional. Pass `--dev` or `--prod` through to `pnpm outdated` to scope the check. + +## 1. Pre-flight + +1. `git status --porcelain` — if there are uncommitted changes touching `package.json` or `pnpm-lock.yaml`, STOP and tell me to commit or stash first. Other unrelated WIP is fine but warn me. +2. Confirm this is a pnpm workspace by reading `pnpm-workspace.yaml` (or at least `package.json` with `packageManager: pnpm@*`). If not, stop. + +## 2. Survey + +Run: + +``` +pnpm outdated -r --format json $ARGUMENTS +``` + +If the output is empty / `{}`, say "Everything is up to date." and stop. + +Parse the JSON. For each entry capture: package name, current version, latest version, type (`dependencies` / `devDependencies`), and which workspace(s) it appears in. + +Classify the bump for each package: +- **patch** — `x.y.Z` only changed +- **minor** — `x.Y.z` changed (and major matches) +- **major** — `X.y.z` changed +- **prerelease / weird** — anything else (e.g. `0.x` minor bumps, which are effectively major in semver) + +Treat `0.x` minor bumps as major-equivalent (breaking by convention). + +## 3. Research + +For each **major** and **prerelease/weird** entry, look up release notes before classifying: + +- Try `WebSearch` for ` release notes` or ` changelog `. +- If the package has a known repo, prefer `WebFetch` on `https://github.com///releases/tag/v` or the repo's `CHANGELOG.md`. +- Cap research at ~3 lookups per package; summarize in one line per package. + +For **patch** and **minor** entries, do not research — assume safe unless the name is a well-known framework where minor bumps are routinely breaking (e.g. eslint configs, type-only packages tied to a runtime). Use judgment, note exceptions explicitly. + +## 4. Triage table + +Print one table, grouped by bump class (patch first, then minor, then major): + +``` +PACKAGE CURRENT → LATEST CLASS RECOMMEND NOTES + 1.2.3 → 1.2.7 patch update — + 4.5.0 → 5.0.0 major review drops Node 18; new ESM-only export + 0.3.1 → 0.4.0 major hold breaking type changes, see CHANGELOG +``` + +Recommendation rules: +- `update` — safe to bump now (patch / minor with no known issues). +- `review` — major bump, but release notes look manageable. Worth doing in this pass with awareness. +- `hold` — major bump with breaking changes that need code edits before updating. Do NOT include in the upgrade command this round. + +If you researched a package, the `NOTES` cell must reflect what you actually found. Don't say "see changelog" — summarize the breaking change in ≤ 80 chars. + +## 5. Plan + +Below the table, print the exact command that would run, with packages from rows recommended `update` or `review`: + +``` +pnpm up --latest -r ... +``` + +If the list is empty (only `hold` rows), say so and stop — nothing to upgrade automatically. + +**Gate.** Ask exactly: `Run this update? (yes / edit / cancel)`. Wait for my reply. + +- `yes` → proceed. +- `edit` → let me reclassify rows (e.g. "move to hold", "add back in"), then re-show the command and re-confirm. +- `cancel` → stop without running anything. + +## 6. Apply (only after gate = yes) + +1. Run the confirmed `pnpm up --latest -r ` command. +2. If pnpm errors out, STOP and show the error — do not retry blindly. +3. Run validation, but only scripts that exist in the root `package.json`: + ``` + pnpm typecheck + pnpm lint + pnpm test + pnpm build + ``` + Skip any script that isn't defined. Show pass/fail for each. +4. Show `git diff --stat package.json '**/package.json' pnpm-lock.yaml` so I can see what moved. + +## 7. Summary + +Print: + +- `Updated: Held: Validation: ` +- The exact list of packages that were upgraded, with `old → new` versions. +- If validation failed, the failing script name. Do NOT try to "fix" the failure in this command — that's a separate task. + +Do NOT commit. Do NOT push. Leave the working tree dirty for me to review and commit manually (or via `/commit`). + +## Hard rules + +- Never run `pnpm up` before the gate is confirmed. +- Never include `hold` packages in the upgrade command, even if they're in the same workspace. +- Never edit source code to "fix" a breaking change in this command — flag it and stop. +- Never commit or push. +- Never use `--no-verify` or skip validation. If validation fails, surface it. diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index c097372..7c517bc 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -2,3 +2,5 @@ packages: - "apps/*" - "packages/*" - "tooling/*" + +minimumReleaseAge: 20160 From 84cb11b6972d5d8d5c68fb396c30319c57156cb8 Mon Sep 17 00:00:00 2001 From: Juha Kangas <42040080+valuecodes@users.noreply.github.com> Date: Sun, 26 Apr 2026 11:20:25 +0300 Subject: [PATCH 3/3] feat: confirm schedule actions with inline buttons --- .claude/commands/pnpm-update.md | 2 + apps/operator/README.md | 2 +- .../operator/migrations/0004_minor_rictor.sql | 1 + .../migrations/meta/0004_snapshot.json | 223 ++++++++++++++++ apps/operator/migrations/meta/_journal.json | 7 + apps/operator/scripts/set-webhook.ts | 6 +- apps/operator/src/db/schema.ts | 1 + .../src/modules/telegram/controller.ts | 189 +++++++++++-- .../src/modules/telegram/routes.test.ts | 250 +++++++++++++++++- apps/operator/src/services/pending-action.ts | 99 +++++-- apps/operator/src/services/telegram.test.ts | 77 ++++++ apps/operator/src/services/telegram.ts | 20 +- apps/operator/src/types/telegram.ts | 48 +++- 13 files changed, 868 insertions(+), 57 deletions(-) create mode 100644 apps/operator/migrations/0004_minor_rictor.sql create mode 100644 apps/operator/migrations/meta/0004_snapshot.json diff --git a/.claude/commands/pnpm-update.md b/.claude/commands/pnpm-update.md index 361d284..5dd3387 100644 --- a/.claude/commands/pnpm-update.md +++ b/.claude/commands/pnpm-update.md @@ -26,6 +26,7 @@ If the output is empty / `{}`, say "Everything is up to date." and stop. Parse the JSON. For each entry capture: package name, current version, latest version, type (`dependencies` / `devDependencies`), and which workspace(s) it appears in. Classify the bump for each package: + - **patch** — `x.y.Z` only changed - **minor** — `x.Y.z` changed (and major matches) - **major** — `X.y.z` changed @@ -55,6 +56,7 @@ PACKAGE CURRENT → LATEST CLASS RECOMMEND NOTES ``` Recommendation rules: + - `update` — safe to bump now (patch / minor with no known issues). - `review` — major bump, but release notes look manageable. Worth doing in this pass with awareness. - `hold` — major bump with breaking changes that need code edits before updating. Do NOT include in the upgrade command this round. diff --git a/apps/operator/README.md b/apps/operator/README.md index 0f1be53..5e50ed9 100644 --- a/apps/operator/README.md +++ b/apps/operator/README.md @@ -31,7 +31,7 @@ Run commands from the repo root unless noted otherwise. - The webhook body is limited to 64 KiB and validated against the Telegram update schema - Messages from the allowed Telegram chat are forwarded to OpenAI - The assistant can create, list, and delete scheduled reminders through tool calls -- Schedule creation and deletion require a `YES` confirmation reply within two minutes +- Schedule creation and deletion require confirmation within two minutes via inline ✅/❌ buttons (typing `YES` still works as a fallback) - A cron trigger runs every minute and executes due reminders and web monitors from D1 - Monitor schedules scrape a URL, analyze the content with OpenAI, and notify only when the condition matches diff --git a/apps/operator/migrations/0004_minor_rictor.sql b/apps/operator/migrations/0004_minor_rictor.sql new file mode 100644 index 0000000..4234f71 --- /dev/null +++ b/apps/operator/migrations/0004_minor_rictor.sql @@ -0,0 +1 @@ +ALTER TABLE `pending_actions` ADD `token` text; \ No newline at end of file diff --git a/apps/operator/migrations/meta/0004_snapshot.json b/apps/operator/migrations/meta/0004_snapshot.json new file mode 100644 index 0000000..7e8f867 --- /dev/null +++ b/apps/operator/migrations/meta/0004_snapshot.json @@ -0,0 +1,223 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "258e161f-a05c-4114-9e45-2832a63ff0fa", + "prevId": "32d9e6de-4bae-4cd1-ae78-bdde98e2ec7d", + "tables": { + "pending_actions": { + "name": "pending_actions", + "columns": { + "chat_id": { + "name": "chat_id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "action_type": { + "name": "action_type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "payload": { + "name": "payload", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "expires_at": { + "name": "expires_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "token": { + "name": "token", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "schedules": { + "name": "schedules", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "chat_id": { + "name": "chat_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "schedule_type": { + "name": "schedule_type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "hour": { + "name": "hour", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "minute": { + "name": "minute", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false, + "default": 0 + }, + "day_of_week": { + "name": "day_of_week", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "day_of_month": { + "name": "day_of_month", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "timezone": { + "name": "timezone", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'UTC'" + }, + "fixed_message": { + "name": "fixed_message", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "message_prompt": { + "name": "message_prompt", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "source_url": { + "name": "source_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "keywords": { + "name": "keywords", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "state_json": { + "name": "state_json", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "active": { + "name": "active", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": true + }, + "next_run_at": { + "name": "next_run_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "claimed_at": { + "name": "claimed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "retry_count": { + "name": "retry_count", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "idx_schedules_next_run": { + "name": "idx_schedules_next_run", + "columns": ["active", "next_run_at"], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} diff --git a/apps/operator/migrations/meta/_journal.json b/apps/operator/migrations/meta/_journal.json index 8f94b6a..89ca5b6 100644 --- a/apps/operator/migrations/meta/_journal.json +++ b/apps/operator/migrations/meta/_journal.json @@ -29,6 +29,13 @@ "when": 1777183590996, "tag": "0003_amused_marvex", "breakpoints": true + }, + { + "idx": 4, + "version": "6", + "when": 1777190805045, + "tag": "0004_minor_rictor", + "breakpoints": true } ] } diff --git a/apps/operator/scripts/set-webhook.ts b/apps/operator/scripts/set-webhook.ts index 6791ea0..ea61e19 100644 --- a/apps/operator/scripts/set-webhook.ts +++ b/apps/operator/scripts/set-webhook.ts @@ -81,7 +81,11 @@ const main = async () => { try { const result = await client.post("/setWebhook", { schema: responseSchema, - body: { url: webhookUrl, secret_token: secret }, + body: { + url: webhookUrl, + secret_token: secret, + allowed_updates: ["message", "callback_query"], + }, }); console.log("Response:", JSON.stringify(result, null, 2)); diff --git a/apps/operator/src/db/schema.ts b/apps/operator/src/db/schema.ts index c3ff668..9b3eccf 100644 --- a/apps/operator/src/db/schema.ts +++ b/apps/operator/src/db/schema.ts @@ -40,6 +40,7 @@ const pendingActions = sqliteTable("pending_actions", { payload: text("payload").notNull(), description: text("description").notNull(), expiresAt: text("expires_at").notNull(), + token: text("token"), }); export { pendingActions, schedules }; diff --git a/apps/operator/src/modules/telegram/controller.ts b/apps/operator/src/modules/telegram/controller.ts index ab5731b..8111219 100644 --- a/apps/operator/src/modules/telegram/controller.ts +++ b/apps/operator/src/modules/telegram/controller.ts @@ -12,7 +12,11 @@ import { import type { CreateScheduleInput } from "../../services/schedule"; import { TelegramService } from "../../services/telegram"; import type { AppEnv } from "../../types/env"; -import type { TelegramUpdate } from "../../types/telegram"; +import type { + InlineKeyboardMarkup, + TelegramCallbackQuery, + TelegramUpdate, +} from "../../types/telegram"; import { markdownToTelegramHtml } from "../../utils/markdown-to-html"; import { splitMessage, @@ -68,6 +72,15 @@ const mapToolArgsToInput = ( description: (args.description as string | undefined) ?? "", }); +const buildConfirmationKeyboard = (token: string): InlineKeyboardMarkup => ({ + inline_keyboard: [ + [ + { text: "✅ Yes", callback_data: `c:${token}` }, + { text: "❌ No", callback_data: `x:${token}` }, + ], + ], +}); + const executePendingAction = async ( action: PendingAction, chatId: number, @@ -99,8 +112,18 @@ type WebhookInput = { }; const handleWebhook = async (c: Context) => { - const logger = c.get("logger"); const update = c.req.valid("json"); + if (update.callback_query) { + return handleCallbackQuery(c, update.callback_query); + } + return handleMessage(c, update); +}; + +const handleMessage = async ( + c: Context, + update: TelegramUpdate +) => { + const logger = c.get("logger"); const message = update.message; const isAllowedChat = message !== undefined && String(message.chat.id) === c.env.ALLOWED_CHAT_ID; @@ -132,31 +155,29 @@ const handleWebhook = async (c: Context) => { const telegram = new TelegramService(c.env.TELEGRAM_BOT_TOKEN, logger); const pendingService = new PendingActionService(c.env.DB); - // Check for pending confirmation - const pending = await pendingService.get(chatId); - if (pending) { + // Typed-YES fallback path. Atomically claim any non-expired pending row + // so a concurrent button tap can't also execute it. + const consumed = await pendingService.consumeByChatId(chatId); + if (consumed) { const confirmed = message.text.trim().toUpperCase() === "YES"; if (confirmed) { const result = await executePendingAction( - pending, + consumed, chatId, c.env.DB, logger ); - await pendingService.clear(chatId); for (const chunk of splitMessage(result)) { await telegram.sendMessage({ chat_id: chatId, text: chunk }); } return c.json({ ok: true }); } - await pendingService.clear(chatId); await telegram.sendMessage({ chat_id: chatId, text: "Action cancelled.", }); - // Fall through to process the message normally if it wasn't just "YES"/"NO" const isSimpleResponse = message.text.trim().length <= 3 || message.text.trim().toUpperCase() === "NO"; @@ -167,6 +188,8 @@ const handleWebhook = async (c: Context) => { const scheduleService = new ScheduleService(c.env.DB); + let pendingButtonToken: string | undefined; + const toolExecutor: ToolExecutor = async ( name: string, args: Record @@ -214,15 +237,14 @@ const handleWebhook = async (c: Context) => { } } - // Store as pending in D1 — require user confirmation - await pendingService.set(chatId, { + pendingButtonToken = await pendingService.set(chatId, { type: "create_schedule", payload: input as unknown as Record, description: formatScheduleDescription("create", args), }); return { - result: `Confirmation required. I've asked the user to confirm: "${formatScheduleDescription("create", args)}". Tell the user to reply YES to confirm.`, + result: `Confirmation buttons attached. Reply with a short summary like 'Confirm creating: ${formatScheduleDescription("create", args)}' — do NOT ask the user to type YES.`, }; } @@ -232,15 +254,14 @@ const handleWebhook = async (c: Context) => { return { error: "Missing schedule ID" }; } - // Store as pending in D1 — require user confirmation - await pendingService.set(chatId, { + pendingButtonToken = await pendingService.set(chatId, { type: "delete_schedule", payload: { id }, description: `Delete schedule ${id}`, }); return { - result: `Confirmation required. I've asked the user to confirm deletion of schedule ${id}. Tell the user to reply YES to confirm.`, + result: `Confirmation buttons attached. Reply with a short summary like 'Confirm deleting schedule ${id}' — do NOT ask the user to type YES.`, }; } @@ -259,18 +280,26 @@ const handleWebhook = async (c: Context) => { reply = "Something went wrong while generating a response. Please try again."; replyIsHtml = false; + pendingButtonToken = undefined; await pendingService.clear(chatId); } + const replyMarkup = pendingButtonToken + ? buildConfirmationKeyboard(pendingButtonToken) + : undefined; + try { const chunkMax = replyIsHtml ? TELEGRAM_HTML_SAFE_LENGTH : TELEGRAM_MAX_MESSAGE_LENGTH; - for (const chunk of splitMessage(reply, chunkMax)) { + const chunks = [...splitMessage(reply, chunkMax)]; + for (let i = 0; i < chunks.length; i++) { + const isLast = i === chunks.length - 1; await telegram.sendMessage({ chat_id: chatId, - text: replyIsHtml ? markdownToTelegramHtml(chunk) : chunk, + text: replyIsHtml ? markdownToTelegramHtml(chunks[i]) : chunks[i], ...(replyIsHtml ? { parse_mode: "HTML" as const } : {}), + ...(isLast && replyMarkup ? { reply_markup: replyMarkup } : {}), }); } } catch (error) { @@ -281,4 +310,128 @@ const handleWebhook = async (c: Context) => { return c.json({ ok: true }); }; -export { handleWebhook }; +const parseCallbackData = ( + data: string | undefined +): { action: "confirm" | "cancel"; token: string } | undefined => { + if (!data) { + return undefined; + } + const colon = data.indexOf(":"); + if (colon === -1) { + return undefined; + } + const prefix = data.slice(0, colon); + const token = data.slice(colon + 1); + if (!token) { + return undefined; + } + if (prefix === "c") { + return { action: "confirm", token }; + } + if (prefix === "x") { + return { action: "cancel", token }; + } + return undefined; +}; + +const handleCallbackQuery = async ( + c: Context, + cq: TelegramCallbackQuery +) => { + const logger = c.get("logger"); + const allowedChatId = c.env.ALLOWED_CHAT_ID; + const fromAllowed = String(cq.from.id) === allowedChatId; + + if (!fromAllowed) { + logger.warn("callback_query not allowed, ignoring", { + callbackId: cq.id, + }); + return c.json({ ok: true }); + } + + const cqChatId = cq.message?.chat.id; + if (cqChatId !== undefined && String(cqChatId) !== allowedChatId) { + logger.warn("callback_query not allowed, ignoring", { + callbackId: cq.id, + }); + return c.json({ ok: true }); + } + + const telegram = new TelegramService(c.env.TELEGRAM_BOT_TOKEN, logger); + const messageId = cq.message?.message_id; + const chatId = cq.message?.chat.id; + + if (chatId === undefined) { + // Inaccessible message stub — no chat to act on. Ack and stop. + await telegram + .answerCallbackQuery({ callback_query_id: cq.id }) + .catch(() => undefined); + return c.json({ ok: true }); + } + + const parsed = parseCallbackData(cq.data); + if (!parsed) { + logger.warn("callback_query has malformed data", { callbackId: cq.id }); + await telegram + .answerCallbackQuery({ callback_query_id: cq.id }) + .catch(() => undefined); + return c.json({ ok: true }); + } + + const pendingService = new PendingActionService(c.env.DB); + + let toast: string | undefined; + let resultMessage: string | undefined; + try { + const consumed = await pendingService.consumeByToken(chatId, parsed.token); + if (!consumed) { + toast = "Expired or already used"; + } else if (parsed.action === "cancel") { + toast = "Cancelled"; + } else { + resultMessage = await executePendingAction( + consumed, + chatId, + c.env.DB, + logger + ); + toast = + consumed.type === "create_schedule" + ? "Schedule created" + : "Schedule deleted"; + } + } catch (error) { + logger.error("callback execution failed", { + callbackId: cq.id, + error: error instanceof Error ? error.message : String(error), + }); + toast = "Something went wrong"; + } finally { + await telegram + .answerCallbackQuery({ callback_query_id: cq.id, text: toast }) + .catch((err: unknown) => { + logger.warn("answerCallbackQuery failed", { + error: err instanceof Error ? err.message : String(err), + }); + }); + if (messageId !== undefined) { + await telegram + .editMessageReplyMarkup({ chat_id: chatId, message_id: messageId }) + .catch((err: unknown) => { + logger.warn("editMessageReplyMarkup failed", { + error: err instanceof Error ? err.message : String(err), + }); + }); + } + } + + if (resultMessage) { + for (const chunk of splitMessage(resultMessage)) { + await telegram.sendMessage({ chat_id: chatId, text: chunk }); + } + } + + return c.json({ ok: true }); +}; + +export { handleWebhook, parseCallbackData }; diff --git a/apps/operator/src/modules/telegram/routes.test.ts b/apps/operator/src/modules/telegram/routes.test.ts index cb10c5c..611c0f1 100644 --- a/apps/operator/src/modules/telegram/routes.test.ts +++ b/apps/operator/src/modules/telegram/routes.test.ts @@ -11,13 +11,15 @@ vi.mock("openai", () => ({ }, })); -const pendingGetMock = vi.fn(); +const pendingConsumeByChatIdMock = vi.fn(); +const pendingConsumeByTokenMock = vi.fn(); const pendingClearMock = vi.fn(); const pendingSetMock = vi.fn(); vi.mock("../../services/pending-action", () => ({ PendingActionService: class { - get = pendingGetMock; + consumeByChatId = pendingConsumeByChatIdMock; + consumeByToken = pendingConsumeByTokenMock; clear = pendingClearMock; set = pendingSetMock; }, @@ -65,6 +67,33 @@ const validUpdate = { }, }; +const callbackUpdate = (overrides: { + data?: string; + fromId?: number; + chatId?: number; + messageId?: number; + includeMessage?: boolean; +}) => { + const includeMessage = overrides.includeMessage ?? true; + return { + update_id: 2, + callback_query: { + id: "cb-1", + from: { id: overrides.fromId ?? 12345, is_bot: false, first_name: "U" }, + data: overrides.data ?? "c:tok123", + ...(includeMessage + ? { + message: { + message_id: overrides.messageId ?? 99, + chat: { id: overrides.chatId ?? 12345, type: "private" }, + date: 1234567890, + }, + } + : {}), + }, + }; +}; + const app = new Hono(); app.use("*", loggerMiddleware); app.onError(onErrorHandler); @@ -91,19 +120,23 @@ describe("POST /webhook/telegram", () => { beforeEach(() => { mockFetch.mockReset(); ENV.DB = createMockD1(); - pendingGetMock.mockReset(); - pendingGetMock.mockResolvedValue(undefined); + pendingConsumeByChatIdMock.mockReset(); + pendingConsumeByChatIdMock.mockResolvedValue(undefined); + pendingConsumeByTokenMock.mockReset(); + pendingConsumeByTokenMock.mockResolvedValue(undefined); pendingClearMock.mockReset(); pendingClearMock.mockResolvedValue(undefined); pendingSetMock.mockReset(); - pendingSetMock.mockResolvedValue(undefined); + pendingSetMock.mockResolvedValue("tok-default"); openaiCreateMock.mockResolvedValue({ choices: [{ message: { content: "AI response" } }], }); - mockFetch.mockResolvedValue( - new Response(JSON.stringify({ ok: true }), { - headers: { "Content-Type": "application/json" }, - }) + mockFetch.mockImplementation(() => + Promise.resolve( + new Response(JSON.stringify({ ok: true }), { + headers: { "Content-Type": "application/json" }, + }) + ) ); }); @@ -266,8 +299,8 @@ describe("POST /webhook/telegram", () => { ); }); - it("sends 'Action cancelled.' as plain text without parse_mode", async () => { - pendingGetMock.mockResolvedValueOnce({ + it("sends 'Action cancelled.' as plain text without parse_mode for typed non-YES reply", async () => { + pendingConsumeByChatIdMock.mockResolvedValueOnce({ type: "create_schedule", payload: {}, description: "test action", @@ -384,3 +417,198 @@ describe("POST /webhook/telegram", () => { expect(output).not.toContain("99999"); }); }); + +describe("POST /webhook/telegram — callback_query", () => { + const headers = { + "x-telegram-bot-api-secret-token": ENV.TELEGRAM_WEBHOOK_SECRET, + }; + + beforeEach(() => { + mockFetch.mockReset(); + ENV.DB = createMockD1(); + pendingConsumeByChatIdMock.mockReset(); + pendingConsumeByChatIdMock.mockResolvedValue(undefined); + pendingConsumeByTokenMock.mockReset(); + pendingConsumeByTokenMock.mockResolvedValue(undefined); + pendingClearMock.mockReset(); + pendingClearMock.mockResolvedValue(undefined); + pendingSetMock.mockReset(); + pendingSetMock.mockResolvedValue("tok-default"); + mockFetch.mockImplementation(() => + Promise.resolve( + new Response(JSON.stringify({ ok: true }), { + headers: { "Content-Type": "application/json" }, + }) + ) + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + const fetchedMethods = (): string[] => + mockFetch.mock.calls.map((call) => { + const url = call[0] as string; + return url.split("/").pop() ?? ""; + }); + + it("ignores callback_query from disallowed user", async () => { + const res = await sendRequest( + callbackUpdate({ fromId: 99999, data: "c:tok-x" }), + headers + ); + + expect(res.status).toBe(200); + expect(mockFetch).not.toHaveBeenCalled(); + expect(pendingConsumeByTokenMock).not.toHaveBeenCalled(); + }); + + it("ignores callback_query from disallowed chat", async () => { + const res = await sendRequest( + callbackUpdate({ chatId: 99999, data: "c:tok-x" }), + headers + ); + + expect(res.status).toBe(200); + expect(mockFetch).not.toHaveBeenCalled(); + expect(pendingConsumeByTokenMock).not.toHaveBeenCalled(); + }); + + it("acks but does nothing when callback has no message field", async () => { + const res = await sendRequest( + callbackUpdate({ includeMessage: false, data: "c:tok-x" }), + headers + ); + + expect(res.status).toBe(200); + expect(pendingConsumeByTokenMock).not.toHaveBeenCalled(); + expect(fetchedMethods()).toContain("answerCallbackQuery"); + expect(fetchedMethods()).not.toContain("editMessageReplyMarkup"); + expect(fetchedMethods()).not.toContain("sendMessage"); + }); + + it("acks and ignores malformed callback_data", async () => { + const res = await sendRequest(callbackUpdate({ data: "garbage" }), headers); + + expect(res.status).toBe(200); + expect(pendingConsumeByTokenMock).not.toHaveBeenCalled(); + expect(fetchedMethods()).toContain("answerCallbackQuery"); + }); + + it("expired/unknown token: acks with toast and clears buttons, no execution", async () => { + pendingConsumeByTokenMock.mockResolvedValueOnce(undefined); + + const res = await sendRequest( + callbackUpdate({ data: "c:tok-stale" }), + headers + ); + + expect(res.status).toBe(200); + const ackCall = mockFetch.mock.calls.find( + (c) => typeof c[0] === "string" && c[0].endsWith("/answerCallbackQuery") + ); + expect(ackCall).toBeDefined(); + if (!ackCall) { + return; + } + const ackBody = JSON.parse((ackCall[1] as { body: string }).body) as Record< + string, + unknown + >; + expect(ackBody.text).toBe("Expired or already used"); + + const editCall = mockFetch.mock.calls.find( + (c) => + typeof c[0] === "string" && c[0].endsWith("/editMessageReplyMarkup") + ); + expect(editCall).toBeDefined(); + const sendCall = mockFetch.mock.calls.find( + (c) => typeof c[0] === "string" && c[0].endsWith("/sendMessage") + ); + expect(sendCall).toBeUndefined(); + }); + + it("cancel: acks with 'Cancelled', clears buttons, no execution", async () => { + pendingConsumeByTokenMock.mockResolvedValueOnce({ + type: "create_schedule", + payload: {}, + description: "any", + }); + + const res = await sendRequest(callbackUpdate({ data: "x:tok-1" }), headers); + + expect(res.status).toBe(200); + expect(pendingConsumeByTokenMock).toHaveBeenCalledWith(12345, "tok-1"); + const ackCall = mockFetch.mock.calls.find( + (c) => typeof c[0] === "string" && c[0].endsWith("/answerCallbackQuery") + ); + expect(ackCall).toBeDefined(); + if (!ackCall) { + return; + } + const ackBody = JSON.parse((ackCall[1] as { body: string }).body) as Record< + string, + unknown + >; + expect(ackBody.text).toBe("Cancelled"); + const sendCall = mockFetch.mock.calls.find( + (c) => typeof c[0] === "string" && c[0].endsWith("/sendMessage") + ); + expect(sendCall).toBeUndefined(); + }); + + it("confirm delete: acks 'Schedule deleted' and sends result message", async () => { + pendingConsumeByTokenMock.mockResolvedValueOnce({ + type: "delete_schedule", + payload: { id: "abc" }, + description: "Delete schedule abc", + }); + const removeMock = vi.fn().mockResolvedValue(true); + const { ScheduleService } = await import("../../services/schedule"); + vi.spyOn(ScheduleService.prototype, "remove").mockImplementation( + removeMock + ); + + const res = await sendRequest(callbackUpdate({ data: "c:tok-1" }), headers); + + expect(res.status).toBe(200); + expect(removeMock).toHaveBeenCalledWith("abc", 12345); + const ackCall = mockFetch.mock.calls.find( + (c) => typeof c[0] === "string" && c[0].endsWith("/answerCallbackQuery") + ); + expect(ackCall).toBeDefined(); + if (!ackCall) { + return; + } + const ackBody = JSON.parse((ackCall[1] as { body: string }).body) as Record< + string, + unknown + >; + expect(ackBody.text).toBe("Schedule deleted"); + const sendCall = mockFetch.mock.calls.find( + (c) => typeof c[0] === "string" && c[0].endsWith("/sendMessage") + ); + expect(sendCall).toBeDefined(); + }); + + it("double-tap: second consumeByToken returns undefined → no double execution", async () => { + pendingConsumeByTokenMock + .mockResolvedValueOnce({ + type: "delete_schedule", + payload: { id: "abc" }, + description: "Delete schedule abc", + }) + .mockResolvedValueOnce(undefined); + const removeMock = vi.fn().mockResolvedValue(true); + const { ScheduleService } = await import("../../services/schedule"); + vi.spyOn(ScheduleService.prototype, "remove").mockImplementation( + removeMock + ); + + await sendRequest(callbackUpdate({ data: "c:tok-1" }), headers); + await sendRequest(callbackUpdate({ data: "c:tok-1" }), headers); + + expect(removeMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/operator/src/services/pending-action.ts b/apps/operator/src/services/pending-action.ts index 806c6c3..435d4dc 100644 --- a/apps/operator/src/services/pending-action.ts +++ b/apps/operator/src/services/pending-action.ts @@ -1,4 +1,4 @@ -import { eq, lte } from "drizzle-orm"; +import { and, eq, gt, lte } from "drizzle-orm"; import { drizzle } from "drizzle-orm/d1"; import type { DrizzleD1Database } from "drizzle-orm/d1"; @@ -14,6 +14,29 @@ type PendingAction = { const TTL_MS = 2 * 60 * 1000; +const generateToken = (): string => { + const bytes = new Uint8Array(16); + crypto.getRandomValues(bytes); + let binary = ""; + for (const byte of bytes) { + binary += String.fromCharCode(byte); + } + return btoa(binary) + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/, ""); +}; + +const rowToAction = (row: { + actionType: PendingActionType; + payload: string; + description: string; +}): PendingAction => ({ + type: row.actionType, + payload: JSON.parse(row.payload) as Record, + description: row.description, +}); + class PendingActionService { private readonly db: DrizzleD1Database; @@ -21,8 +44,14 @@ class PendingActionService { this.db = drizzle(d1); } - async set(chatId: number, action: PendingAction): Promise { + /** + * Stores the action and returns a token that must be embedded in the + * inline-button callback_data. Old rows for this chat are overwritten, + * which also invalidates any token bound to the previous row. + */ + async set(chatId: number, action: PendingAction): Promise { const expiresAt = new Date(Date.now() + TTL_MS).toISOString(); + const token = generateToken(); await this.db .insert(pendingActions) .values({ @@ -31,6 +60,7 @@ class PendingActionService { payload: JSON.stringify(action.payload), description: action.description, expiresAt, + token, }) .onConflictDoUpdate({ target: pendingActions.chatId, @@ -39,34 +69,59 @@ class PendingActionService { payload: JSON.stringify(action.payload), description: action.description, expiresAt, + token, }, }); + return token; } - async get(chatId: number): Promise { - const rows = await this.db - .select() - .from(pendingActions) - .where(eq(pendingActions.chatId, chatId)) - .limit(1); - - if (rows.length === 0) { + /** + * Atomically claims the pending action by token in a single DELETE, + * preventing double-execution when two callbacks (e.g. a double-tap) + * race. A stale token simply doesn't match and leaves the current row + * intact — only expiry clears. + */ + async consumeByToken( + chatId: number, + token: string + ): Promise { + const nowIso = new Date().toISOString(); + const deleted = await this.db + .delete(pendingActions) + .where( + and( + eq(pendingActions.chatId, chatId), + eq(pendingActions.token, token), + gt(pendingActions.expiresAt, nowIso) + ) + ) + .returning(); + if (deleted.length === 0) { return undefined; } + return rowToAction(deleted[0]); + } - const row = rows[0]; - - // Check expiry - if (new Date(row.expiresAt) <= new Date()) { - await this.clear(chatId); + /** + * Atomically claims the pending action for a chat without a token check, + * used by the typed-YES fallback path. Returns undefined when no + * non-expired row exists. + */ + async consumeByChatId(chatId: number): Promise { + const nowIso = new Date().toISOString(); + const deleted = await this.db + .delete(pendingActions) + .where( + and( + eq(pendingActions.chatId, chatId), + gt(pendingActions.expiresAt, nowIso) + ) + ) + .returning(); + if (deleted.length === 0) { return undefined; } - - return { - type: row.actionType, - payload: JSON.parse(row.payload) as Record, - description: row.description, - }; + return rowToAction(deleted[0]); } async clear(chatId: number): Promise { @@ -83,5 +138,5 @@ class PendingActionService { } } -export { PendingActionService, TTL_MS }; +export { generateToken, PendingActionService, TTL_MS }; export type { PendingAction, PendingActionType }; diff --git a/apps/operator/src/services/telegram.test.ts b/apps/operator/src/services/telegram.test.ts index dad0da2..57bed11 100644 --- a/apps/operator/src/services/telegram.test.ts +++ b/apps/operator/src/services/telegram.test.ts @@ -88,6 +88,7 @@ describe("TelegramService", () => { body: JSON.stringify({ url: "https://example.com/webhook", secret_token: "my-secret", + allowed_updates: ["message", "callback_query"], }), } ); @@ -101,4 +102,80 @@ describe("TelegramService", () => { expect(result).toEqual({ ok: true }); }); }); + + describe("answerCallbackQuery", () => { + it("posts to /answerCallbackQuery with the given params", async () => { + mockFetch.mockResolvedValueOnce(createJsonResponse({ ok: true })); + + await service.answerCallbackQuery({ + callback_query_id: "cb-1", + text: "Done", + }); + + expect(mockFetch).toHaveBeenCalledWith( + `https://api.telegram.org/bot${token}/answerCallbackQuery`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ callback_query_id: "cb-1", text: "Done" }), + } + ); + }); + }); + + describe("editMessageReplyMarkup", () => { + it("posts to /editMessageReplyMarkup with chat and message ids", async () => { + mockFetch.mockResolvedValueOnce(createJsonResponse({ ok: true })); + + await service.editMessageReplyMarkup({ chat_id: 1, message_id: 2 }); + + expect(mockFetch).toHaveBeenCalledWith( + `https://api.telegram.org/bot${token}/editMessageReplyMarkup`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ chat_id: 1, message_id: 2 }), + } + ); + }); + }); + + describe("sendMessage with reply_markup", () => { + it("includes inline keyboard in the body", async () => { + mockFetch.mockResolvedValueOnce(createJsonResponse({ ok: true })); + + await service.sendMessage({ + chat_id: 1, + text: "Confirm?", + reply_markup: { + inline_keyboard: [ + [ + { text: "Yes", callback_data: "c:t1" }, + { text: "No", callback_data: "x:t1" }, + ], + ], + }, + }); + + expect(mockFetch).toHaveBeenCalledWith( + `https://api.telegram.org/bot${token}/sendMessage`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + chat_id: 1, + text: "Confirm?", + reply_markup: { + inline_keyboard: [ + [ + { text: "Yes", callback_data: "c:t1" }, + { text: "No", callback_data: "x:t1" }, + ], + ], + }, + }), + } + ); + }); + }); }); diff --git a/apps/operator/src/services/telegram.ts b/apps/operator/src/services/telegram.ts index 9f5f82c..1913f12 100644 --- a/apps/operator/src/services/telegram.ts +++ b/apps/operator/src/services/telegram.ts @@ -1,7 +1,11 @@ import { HttpClient } from "@repo/http-client"; import type { Logger } from "@repo/logger"; -import type { SendMessageParams } from "../types/telegram"; +import type { + AnswerCallbackQueryParams, + EditMessageReplyMarkupParams, + SendMessageParams, +} from "../types/telegram"; import { telegramApiResponseSchema } from "../types/telegram"; class TelegramService { @@ -33,7 +37,19 @@ class TelegramService { } async setWebhook(url: string, secretToken?: string) { - return this.request("setWebhook", { url, secret_token: secretToken }); + return this.request("setWebhook", { + url, + secret_token: secretToken, + allowed_updates: ["message", "callback_query"], + }); + } + + async answerCallbackQuery(params: AnswerCallbackQueryParams) { + return this.request("answerCallbackQuery", params); + } + + async editMessageReplyMarkup(params: EditMessageReplyMarkupParams) { + return this.request("editMessageReplyMarkup", params); } } diff --git a/apps/operator/src/types/telegram.ts b/apps/operator/src/types/telegram.ts index 74e67be..167c0b4 100644 --- a/apps/operator/src/types/telegram.ts +++ b/apps/operator/src/types/telegram.ts @@ -21,17 +21,49 @@ const telegramMessageSchema = z }) .loose(); +// `message` is optional because Telegram may deliver an "inaccessible +// message" stub for very old buttons; keys we don't use are also tolerated. +const telegramCallbackQuerySchema = z + .object({ + id: z.string(), + from: telegramUserSchema, + message: telegramMessageSchema.optional(), + data: z.string().optional(), + }) + .loose(); + const telegramUpdateSchema = z .object({ update_id: z.number(), message: telegramMessageSchema.optional(), + callback_query: telegramCallbackQuerySchema.optional(), }) .loose(); +const inlineKeyboardButtonSchema = z.object({ + text: z.string(), + callback_data: z.string().max(64), +}); + +const inlineKeyboardMarkupSchema = z.object({ + inline_keyboard: z.array(z.array(inlineKeyboardButtonSchema)), +}); + const sendMessageParamsSchema = z.object({ chat_id: z.number(), text: z.string(), parse_mode: z.enum(["HTML", "MarkdownV2"]).optional(), + reply_markup: inlineKeyboardMarkupSchema.optional(), +}); + +const answerCallbackQueryParamsSchema = z.object({ + callback_query_id: z.string(), + text: z.string().optional(), +}); + +const editMessageReplyMarkupParamsSchema = z.object({ + chat_id: z.number(), + message_id: z.number(), }); const telegramApiResponseSchema = z @@ -42,12 +74,24 @@ const telegramApiResponseSchema = z .loose(); export { - telegramUpdateSchema, - telegramMessageSchema, + answerCallbackQueryParamsSchema, + editMessageReplyMarkupParamsSchema, + inlineKeyboardMarkupSchema, sendMessageParamsSchema, telegramApiResponseSchema, + telegramCallbackQuerySchema, + telegramMessageSchema, + telegramUpdateSchema, }; export type TelegramUpdate = z.infer; export type TelegramMessage = z.infer; +export type TelegramCallbackQuery = z.infer; export type SendMessageParams = z.infer; +export type AnswerCallbackQueryParams = z.infer< + typeof answerCallbackQueryParamsSchema +>; +export type EditMessageReplyMarkupParams = z.infer< + typeof editMessageReplyMarkupParamsSchema +>; +export type InlineKeyboardMarkup = z.infer;