Skip to content

Fix Telegram MCP error boundaries#12

Open
speech115 wants to merge 1 commit into
mainfrom
codex/thermonuclear-fixes
Open

Fix Telegram MCP error boundaries#12
speech115 wants to merge 1 commit into
mainfrom
codex/thermonuclear-fixes

Conversation

@speech115

Copy link
Copy Markdown
Owner

Summary

  • split MCP transport failures from semantic tool errors so contract errors do not trigger endpoint failover
  • centralize contract-error detection in errors.py and collapse fast_read_today onto the shared HTTP client
  • unify tg CLI telemetry/envelopes for success and real McpToolError paths
  • keep the existing confirmation, facade, member-export, runtime, and docs hardening changes

Verification

  • PYTHONPATH=/Users/sereja/.config/superpowers/worktrees/telegram/codex-thermonuclear-fixes/mcp/src /Users/sereja/Projects/tools/telegram/mcp/.venv/bin/python -m unittest discover -s tests -p 'test_*.py' -q\n- PYTHONPATH=/Users/sereja/.config/superpowers/worktrees/telegram/codex-thermonuclear-fixes/mcp/src /Users/sereja/Projects/tools/telegram/mcp/.venv/bin/python -m compileall -q src tests\n- python3 -m pytest control-plane/tests/test_command_registry.py control-plane/tests/test_surface_docs.py -q

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 494c5b77c1

ℹ️ 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".

store.approve(token)
return "<h1>Одобрено</h1><p class='meta'>Можно отправлять через telegram_confirmed_send.</p>"
if action == "reject":
store.reject(token)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Не позволяйте повторному POST отменять одобрение

Если открыты две страницы approval или браузер повторно отправит старую форму с тем же nonce, этот путь вызывает store.reject() даже для записи, которая уже была approved; SendConfirmationStore.reject() не запрещает состояние approved, поэтому ранее одобренный токен перейдет в rejected, и следующий telegram_confirmed_send получит confirmation_rejected. До изменения GET-обработчик выполнял approve/reject только для pending, поэтому здесь нужно снова проверять состояние или делать проверку и мутацию атомарно.

Useful? React with 👍 / 👎.

timeout=timeout,
)
except McpToolError as exc:
raise FastReadError(str(exc)) from None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Сохраняйте McpToolError для fast-read ошибок

Когда telegram_read возвращает typed contract error, call_tool_once уже поднимает McpToolError с payload, но здесь он превращается в обычный FastReadError(str(exc)); из-за префикса вроде MCP tool error at http://... exception_is_tool_error() больше не распознает коды permission_denied/rate_limited. В результате telegram-fast-read-today --json для таких MCP-ошибок выдает generic error: <string> вместо стабильного error: telegram_tool_error, что ломает вызывающих, которые ветвятся по коду ошибки; лучше пробросить McpToolError или сохранить код/payload.

Useful? React with 👍 / 👎.

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