fix(telegram): keychain fallback + preflight SSE/user-config detection (v2.0.7)#185
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 6 minutes and 7 seconds.Comment |
…rces Setup wizard previously re-prompted for Slack/Telegram even when fully configured, because preflight only checked macOS keychain. In practice, credentials may live in: - ~/.claude/plugins/data/ops-ops-marketplace/user-config.json (Telegram api_id/api_hash/phone/session set via plugin user_config) - SSE router on http://127.0.0.1:8090/servers/<name>/sse (auth held server-side; ~/.claude.json mcpServers.* shows {type:sse,url:...}) Changes: - bin/ops-setup-preflight: probe SSE router URL and write source:sse_router to slack.json/telegram.txt; fall back to user-config.json for Telegram - bin/ops-setup-detect: emit channels.<svc>.status=configured when preflight reports SSE router OK or user-config.json has all 4 telegram keys - skills/setup/SKILL.md: extend Universal Credential Auto-Scan to source 11 (user-config.json) and source 12 (SSE router probe); document detection short-circuit so 3a/3d skip re-setup when already wired - agents/doctor-agent.md: cross-reference new detection sources in audit flow - tests/test-preflight-sse-userconfig.sh: 9 cases covering SSE 200/000, user-config full/partial, and detect short-circuit Reproduction (before patch): preferences.json → channels.slack.status=configured (source:keychain) preflight → slack.json {"ok":false,"reason":"no_keychain_tokens"} → setup wizard re-prompts Slack on every run
530fbed to
d525b01
Compare
| [ -n "$tg_phone" ] && count=$((count+1)) | ||
| [ -n "$tg_session" ] && count=$((count+1)) | ||
| TELEGRAM_KEYS_FOUND=$count | ||
| if [ "$count" -eq 4 ]; then | ||
| TELEGRAM_STATUS="configured" | ||
| TELEGRAM_SOURCE="user_config" | ||
| elif [ "$count" -gt 0 ]; then | ||
| TELEGRAM_STATUS="partial" | ||
| TELEGRAM_SOURCE="user_config" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Bug: The live-detection else block omits the keychain fallback for Telegram credentials, causing the script to incorrectly report 'unconfigured' when the preflight cache is unavailable.
Severity: MEDIUM
Suggested Fix
In the else block of the live-detection logic (lines 161-199), add a call to cred_get or a similar function to check for and retrieve Telegram credentials from the keychain. This will ensure the keychain is used as a fallback, consistent with the documented resolution order.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: claude-ops/bin/ops-setup-detect#L161-L198
Potential issue: In the live-detection logic, when the preflight cache is not available,
the `else` block (lines 161-199) checks for the SSE router and `user-config.json` but
fails to check for Telegram credentials in the keychain. This contradicts the documented
resolution order mentioned in a comment at line 134. As a result, under conditions like
a system reboot (which clears `/tmp`) or direct script execution, existing keychain
credentials will be ignored, leading to a false 'unconfigured' status and unnecessarily
re-prompting the user for credentials.
Did we get this right? 👍 / 👎 to inform future reviews.
The plugin's .mcp.json injects TELEGRAM_API_ID/HASH/SESSION/PHONE via
${user_config.telegram_*} placeholders, which resolve to empty strings
when the user configured Telegram via /ops:setup (writes to keychain),
not the plugin settings UI. Result: a working keychain integration
appeared "not configured" because the MCP server wouldn't start.
telegram-server/index.js now falls back to:
security find-generic-password -s telegram-{api-id,api-hash,session,phone} -w
when the env var is empty. macOS-only — Linux/Windows users continue
via env vars or settings UI.
Also bumps marketplace.json + plugin.json to 2.0.7 and adds CHANGELOG
entry covering this fix + the rebased preflight SSE/user-config
detection improvements.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
d525b01 to
2e1c865
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed:
grep -cwith|| echo 0produces corrupted value- Replaced command substitution so grep’s exit 1 on zero matches assigns TELEGRAM_KEYS_FOUND=0 outside $(), avoiding concatenated "0\n0" and valid JSON.
You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 6e494cc. Configure here.
| TELEGRAM_KEYS_FOUND=4 | ||
| else | ||
| # Count keys with "found" in their value | ||
| TELEGRAM_KEYS_FOUND=$(grep -c '=found' "$PREFLIGHT/telegram.txt" 2>/dev/null || echo 0) |
There was a problem hiding this comment.
grep -c with || echo 0 produces corrupted value
High Severity
When grep -c '=found' matches zero lines, it writes "0" to stdout and exits with code 1. Because || echo 0 is inside the $() command substitution, both outputs are captured, setting TELEGRAM_KEYS_FOUND to "0\n0" (two zeros separated by a newline). This corrupts the downstream [ -eq 4 ] / [ -gt 0 ] integer comparisons (bash emits "integer expression expected") and produces invalid JSON at "keys_found": $TELEGRAM_KEYS_FOUND. Moving the fallback outside the substitution (e.g. TELEGRAM_KEYS_FOUND=$(grep -c … 2>/dev/null) || TELEGRAM_KEYS_FOUND=0) avoids double capture.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6e494cc. Configure here.
…release (#199) The v2.0.5 → v2.0.9 patch series shipped meaningful features (multi-workspace Slack #195, /ops:credentials audit #184, ops-ci current-state filter #196, telegram preflight #185, userConfig schema upgrades #182). Per semver these should have been a minor bump. This release retroactively rolls them up into v2.1.0 with a single coherent CHANGELOG entry. No code changes — only: - plugin.json: 2.0.9 → 2.1.0 - CHANGELOG.md: new [2.1.0] entry consolidating Added/Fixed/Notes for the patch series - README header + What's-new section: refer to v2.1.0 - 11 docs/*.md badges + agents-reference subtitle + migration latest-stable note: v2.0.9 → v2.1.0 Marketplace pin (.claude-plugin/marketplace.json) bumped in follow-up PR. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Two bundled fixes shipping as v2.0.7:
1. Telegram MCP server now falls back to macOS Keychain at startup
The plugin's
.mcp.jsoninjectsTELEGRAM_API_ID/HASH/SESSION/PHONEvia${user_config.telegram_*}placeholders. When users configure Telegram via/ops:setup(which writes to Keychain) instead of pasting into the plugin settings UI, those placeholders resolve to empty strings and the MCP server fails to start — a working integration appeared "not configured".Patch:
telegram-server/index.jsnow reads:when each env var is empty. macOS-only fallback; Linux/Windows users keep using env vars or the settings UI.
Verified live: keychain probe returns
FOUNDfor all 4 fields on the dev machine.2. Preflight detects SSE-router + user-config.json credential sources
(Rebased onto current main from earlier branch — original commit
20821ee.) Adds two new credential scout sources to setup preflight: SSE router config and${CLAUDE_PLUGIN_DATA_DIR}/user-config.json. New test intests/test-preflight-sse-userconfig.sh. Updatesbin/ops-setup-detect.Test plan
node --check telegram-server/index.jsbash -npasses on all bin/ + scripts/npx prettier --check '**/*.{js,mjs,json}'— all matched files use Prettier code styleNote
Medium Risk
Medium risk because it changes credential-resolution behavior (Keychain reads, SSE-router probing, and new config sources) in setup/detection paths and could misclassify channel configuration or introduce platform-specific edge cases.
Overview
Ships v
2.0.7with more robust Telegram/Slack credential detection during setup and runtime.The Telegram MCP server now falls back to reading credentials from macOS Keychain when env vars are empty, avoiding false “not configured” starts when
/ops:setupstored Telegram secrets in Keychain.Setup preflight/detection now recognizes SSE-router–backed MCP servers (by probing
~/.claude.jsonmcpServers.<svc>.type=="sse"URLs) and a persisted${CLAUDE_PLUGIN_DATA_DIR}/user-config.jsonfallback for Telegram, surfaces this aschannels.{slack,telegram}status/source inops-setup-detect, and adds a new bash test covering these paths. Also updates docs/agent guidance, bumps plugin/marketplace versions, and allowliststests/in gitleaks.Reviewed by Cursor Bugbot for commit 6e494cc. Bugbot is set up for automated code reviews on this repo. Configure here.