Confirm schedule create/delete with inline Telegram buttons - #12
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the operator Telegram bot to use inline ✅/❌ buttons (with token-backed replay protection) for schedule create/delete confirmation, while keeping typed YES as a fallback. This modernizes the confirmation UX and adds the necessary Telegram API/supporting DB changes.
Changes:
- Add Telegram
callback_querysupport (types + webhook handling) and new Telegram API helpers for answering callbacks and clearing inline keyboards. - Replace pending-action confirmation reads with atomic “consume” deletes and add a server-issued token stored in D1 for replay protection.
- Add D1 migration to persist the pending-action token; update webhook tooling and documentation accordingly.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds workspace-level minimumReleaseAge configuration. |
| apps/operator/src/types/telegram.ts | Extends Telegram Zod schemas/types to include callback queries and inline keyboards. |
| apps/operator/src/services/telegram.ts | Adds allowed_updates to webhook registration and introduces answerCallbackQuery / editMessageReplyMarkup. |
| apps/operator/src/services/telegram.test.ts | Adds coverage for the new Telegram service methods and reply_markup support. |
| apps/operator/src/services/pending-action.ts | Adds token generation + atomic consume-by-token / consume-by-chat-id flows. |
| apps/operator/src/modules/telegram/routes.test.ts | Adds callback_query route tests and updates pending-action mocks for new consume APIs. |
| apps/operator/src/modules/telegram/controller.ts | Implements callback_query handler, inline button confirmation flow, and typed-YES fallback via atomic consume. |
| apps/operator/src/db/schema.ts | Adds nullable token column to pending_actions. |
| apps/operator/scripts/set-webhook.ts | Registers webhook with allowed_updates including callback_query. |
| apps/operator/migrations/meta/_journal.json | Records new migration metadata. |
| apps/operator/migrations/meta/0004_snapshot.json | Adds updated schema snapshot including pending_actions.token. |
| apps/operator/migrations/0004_minor_rictor.sql | Adds token column to pending_actions. |
| apps/operator/README.md | Updates documentation to reflect inline-button confirmations and typed-YES fallback. |
| .claude/commands/review-plan.md | Adds a new slash command doc for reviewing plan/design docs. |
| .claude/commands/pnpm-update.md | Adds a new slash command doc for triaging/updating pnpm dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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 }); |
There was a problem hiding this comment.
Callback queries that fail the allowlist check return without calling answerCallbackQuery. Telegram clients will keep showing a loading spinner until the callback is acknowledged, which contradicts the PR’s goal of always acking callbacks. Consider answering the callback query (optionally without text) before returning on the not-allowed path.
There was a problem hiding this comment.
Intentional — unauthorized callbacks are silently dropped here, mirroring the message path (controller.ts:145-148). The spinner on an untrusted client is acceptable cost for not acknowledging unauthorized input.
| 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 }); | ||
| } |
There was a problem hiding this comment.
Callback queries with a message.chat.id that doesn’t match ALLOWED_CHAT_ID are ignored without calling answerCallbackQuery. Even though the request is rejected logically, the Telegram client still needs the callback to be acknowledged to stop the “loading” state. Suggest acking the callback query before returning here as well.
There was a problem hiding this comment.
Same intent as the from.id branch above — disallowed chats get the same silent drop the message path uses, so we're not acking on the unauthorized path.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84cb11b697
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| payload: JSON.stringify(action.payload), | ||
| description: action.description, | ||
| expiresAt, | ||
| token, |
There was a problem hiding this comment.
PendingActionService.set now unconditionally writes the new token column, so any instance running this commit against a DB that has not applied 0004_minor_rictor.sql will throw no such column: token on schedule create/delete. I checked the repo workflows: deployment runs on every main push in .github/workflows/main.yml while migrations run in a separate path-filtered workflow (.github/workflows/migrations.yml), so code can be live before the schema change. This makes confirmations fail in production during rollout unless migration ordering is enforced or writes are made backward-compatible.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Acknowledged — keeping the workflows split for now. Operationally we land migrations ahead of code that depends on the new column; if this becomes a recurring footgun we'll fold the migrate step into the deploy job.
What
YESconfirmation flow for schedule create/delete with one-tap ✅/❌ inline buttons; typedYESremains as a fallback.ALLOWED_CHAT_IDis enforced on the new callback path too.tokencolumn topending_action./pnpm-updateand/review-planslash commands; sets pnpmminimumReleaseAgeto 2 weeks.How to test
pnpm --filter @repo/operator db:migrate:local
pnpm typecheck
pnpm lint
pnpm test
pnpm format:check
Recommended: in the allowed Telegram chat, ask the bot to create / delete a schedule, then exercise: ✅ tap, ❌ tap, > 2 min expiry, and typed-
YESfallback. Verify toast appears and buttons clear after each tap.Security review
ALLOWED_CHAT_IDallowlist as the message path; callbacks without achat.idare acknowledged but ignored.tokencolumn topending_action(server-issued opaque string, no PII).