Skip to content

feat(desktop): add Telegram feedback modal - #918

Draft
alexanderludwig wants to merge 4 commits into
mainfrom
codex/telegram-feedback-help
Draft

feat(desktop): add Telegram feedback modal#918
alexanderludwig wants to merge 4 commits into
mainfrom
codex/telegram-feedback-help

Conversation

@alexanderludwig

Copy link
Copy Markdown
Collaborator

Summary

  • add an Emdash-inspired Give feedback action to Desktop's Help & Support page
  • submit text, optional email, images, and opt-in privacy-redacted diagnostics through a dedicated Telegram bot
  • add typed preload/IPC APIs and main-process Telegram multipart/reply-chain delivery
  • bake required release credentials from GitHub secrets while keeping committed defaults null
  • fix the fresh-worktree Desktop bootstrap so CLI dependencies build before the CLI

Behavior

  • validates required feedback text, optional email, up to 10 JPEG/PNG/WebP images, and an 8 MiB attachment budget
  • supports image selection, paste/drop, previews/removal, and Cmd/Ctrl+Enter
  • treats the root Telegram message as authoritative and reports later attachment failures as partial success
  • retains the newest 500 runtime log entries within 512 KiB and redacts credentials, emails, home paths, network addresses, wallets, and peer IDs
  • disables the Help action in builds without feedback configuration

Security

The Telegram bot token is intentionally embedded in public Electron release artifacts and is therefore extractable. The implementation and Desktop README require a dedicated disposable bot with minimal channel permissions and document immediate token rotation. A server-side relay remains the recommended long-term replacement.

No real Telegram credential is committed in this PR.

Validation

  • pnpm -C apps/desktop run ensure:cli-dist
  • pnpm -C apps/desktop run typecheck:renderer
  • pnpm -C apps/desktop test — 267 main-process tests and 332 renderer tests passed
  • pnpm -C apps/desktop run build:renderer
  • git diff --check origin/main
  • committed-source scan found no Telegram bot-token-shaped literals

Renderer build reports only the existing CSS nesting and large-chunk warnings.

Manual QA

Live posting was exercised during local development against a disposable Telegram group. Release-artifact extraction and signed multi-platform release builds were not performed locally.

@alexanderludwig
alexanderludwig marked this pull request as ready for review August 21, 2026 16:13
@alexanderludwig

Copy link
Copy Markdown
Collaborator Author

@claude pls review

@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown

Claude finished @alexanderludwig's task in 2m 1s —— View job


Review

  • Gather context (PR description, changed files)
  • Read key implementation files
  • Review security-sensitive paths (Telegram token baking, config, redaction)
  • Review renderer (FeedbackModal) and IPC/preload surface
  • Post final review

I read through the feedback flow end-to-end: FeedbackModal.tsxpreload.cts/ipc/feedback.tsfeedback/service.tstelegram/bot-api.ts, plus feedback/config.ts, feedback/diagnostic-log.ts, and the release workflow/bake script changes. Overall this is clean, well-tested (validation, redaction, and partial-failure paths all have dedicated unit tests), and the security tradeoff around the embedded bot token is explicitly called out and documented rather than glossed over.

Worth confirming before merge — release CI will hard-fail without new secrets
.github/workflows/release-desktop.yml now runs node scripts/bake-feedback-telegram-config.mjs --require on all three platform jobs (mac/win/linux). Per scripts/bake-feedback-telegram-config.mjs:36-44, --require causes the script to process.exit(1) if ANTSEED_FEEDBACK_TELEGRAM_BOT_TOKEN/ANTSEED_FEEDBACK_TELEGRAM_CHAT_ID aren't set. If those repo secrets aren't added before this merges (or before the next desktop-v* tag push), every desktop release build breaks at that step, not just the feedback feature. Worth double-checking the secrets are provisioned as part of landing this.

Minor — feedback message isn't chunked for Telegram's 4096-char limit
bot-api.ts already exports splitTelegramMessage() (used by telegram/bridge.ts), but submitTelegramFeedback in feedback/service.ts:158 calls client.sendMessage directly with the unsplit formatFeedbackMessage(...) output. The existing test (service.test.ts:55-64) confirms the max-length case (3,000-char feedback + max email + a modest platform string) stays under 4,096 and passes today, but the platform line (${process.platform} ${process.arch} ${release()}) is not bounded — an unusually long os.release() string on some platform/OS combo could push the root message over Telegram's limit and fail the whole submission (including images/diagnostics, which are attached as replies to that root message). Low risk given today's inputs, but reusing splitTelegramMessage (or truncating the platform string) would remove the last bit of unbounded input from that message.

Nit — duplicated EMAIL_PATTERN / max-length constants
EMAIL_PATTERN is defined identically in both FeedbackModal.tsx:31 and service.ts:21. This matches the existing pattern of duplicating types across the sandboxed preload/renderer boundary elsewhere in this codebase (see the TelegramBridgeStatus/ChatAiStreamStopReason comments in preload.cts), so I wouldn't block on it, just flagging since unlike those cases this one isn't commented as "keep in sync."

What looks solid

  • validateFeedbackRequest (service.ts:59) revalidates everything client-side validation already checked (text length, email format, image count/size/MIME, base64 well-formedness, and that decoded byte length matches the claimed size) — good defense against a compromised/buggy renderer.
  • safeFilename strips path separators and constrains the character set before any filename reaches the Telegram multipart upload.
  • StableRedactor in diagnostic-log.ts correctly orders the wallet-address regex (0x + 40 hex) before the generic 40-hex "peer ID" regex, so wallet addresses are consumed first and don't double-match.
  • The "root message authoritative, attachments best-effort" behavior described in the PR body is exactly what's implemented — image/document upload failures produce attachmentWarnings rather than failing the whole submission, and this is covered by service.test.ts's partial-success test.
  • resolveFeedbackTelegramConfig's runtime-env-vs-baked-defaults precedence logic (config.ts:46-58) is correct and has direct test coverage for the "one of two vars set" partial-config case.

@alexanderludwig
alexanderludwig marked this pull request as draft August 25, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant