feat(channels): outbound send to Telegram/Slack/Discord (CLI + MCP tool) (#152) - #156
Conversation
…ol) (#152) The outward half of channels: a one-shot blocking POST that fits agentflare's sync/ureq model, so an agent can push a message mid-run and the inbound daemon can reuse the same path for replies. - channels.rs: Platform{Telegram,Slack,Discord}; pure build_request() per platform (Telegram token-in-URL {chat_id,text}; Slack Bearer {channel,text}; Discord 'Bot' auth {content}); pure interpret_response() that checks Slack's ok field even on HTTP 200; send()/send_message() resolve the bot token from the encrypted gateway_secrets store and POST via ureq. - cli: 'agentflare channel send --to <platform> --target <id> <message>'. - mcp: channel_send tool so an agent can send during a turn. Tokens are never hardcoded — read from gateway secret '<platform>_bot_token'. Plain-text only for now (no attachments/threads/parse_mode).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds outbound plain-text messaging to Telegram, Slack, and Discord through shared HTTP transport, a CLI command, and an MCP tool. Platform tokens are loaded from encrypted gateway secrets, and platform-specific response rules are applied. ChangesOutbound channel messaging
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI_or_MCP
participant send_message
participant gateway_secrets
participant TelegramSlackDiscord
CLI_or_MCP->>send_message: platform, target, message
send_message->>gateway_secrets: load encrypted bot token
send_message->>TelegramSlackDiscord: send platform-specific POST
TelegramSlackDiscord-->>send_message: status and response body
send_message-->>CLI_or_MCP: success or error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/channels.rs (1)
128-142: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo validation of empty
target/textbefore hitting the network.
send_messageforwards whatevertarget/textit's given straight intobuild_request/send. Neither call site validates non-empty values first:src/cli/channel.rs'sChannelArgs::runandsrc/mcp_server.rs'schannel_sendboth pass user input through untouched (unlike most other MCP tools in this file, e.g.artifact_publish,skill_search, which reject empty required fields withinvalid_paramsbefore doing any I/O). Adding the check once here covers both callers instead of duplicating it.✅ Proposed fix
pub fn send_message( conn: &rusqlite::Connection, platform: Platform, target: &str, text: &str, ) -> Result<(), String> { + if target.trim().is_empty() || text.is_empty() { + return Err("target and message must not be empty".to_string()); + } let name = platform.secret_name();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/channels.rs` around lines 128 - 142, Update send_message to reject empty target or text before retrieving secrets, building the request, or calling send. Return a clear Err describing the missing required value, while preserving the existing secret lookup and network flow for non-empty inputs; this central validation covers ChannelArgs::run and channel_send.src/mcp_server.rs (1)
741-757: 🎯 Functional Correctness | 🔵 Trivial
channel_sendis sync while sibling network-calling tools are async.
gateway_search/gateway_execute(both do outbound network I/O) are declaredasync fn, butchannel_sendcalls a blockingureqrequest synchronously. If the MCP tool dispatcher runs tool methods on a shared async runtime withoutspawn_blocking, this blocking call (compounded by the missing timeout noted insrc/channels.rs::send) could stall other in-flight tool calls. Worth confirming how the#[tool]macro dispatches sync vs async methods before relying on this pattern for a network call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mcp_server.rs` around lines 741 - 757, Update channel_send to avoid performing the blocking channels::send_message network request directly on the async MCP runtime: either make the tool async and run the blocking operation through the runtime’s established spawn_blocking pattern, or use the dispatcher’s supported mechanism for offloading synchronous tools. Preserve the existing platform validation, database error mapping, and response format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/channels.rs`:
- Around line 118-122: Update the error handling in the request send flow to
avoid formatting sensitive req.url in the transport error message. In the Err(e)
branch of the match around r.send_json, use the non-sensitive platform
identifier for context while preserving the existing error propagation behavior.
- Around line 111-124: Update send to use a configured ureq Agent with an
explicit request timeout instead of the default ureq::post client. Preserve the
existing authorization, response-status/body extraction, and interpret_response
behavior while ensuring stalled Slack, Telegram, or Discord requests cannot
block indefinitely.
---
Nitpick comments:
In `@src/channels.rs`:
- Around line 128-142: Update send_message to reject empty target or text before
retrieving secrets, building the request, or calling send. Return a clear Err
describing the missing required value, while preserving the existing secret
lookup and network flow for non-empty inputs; this central validation covers
ChannelArgs::run and channel_send.
In `@src/mcp_server.rs`:
- Around line 741-757: Update channel_send to avoid performing the blocking
channels::send_message network request directly on the async MCP runtime: either
make the tool async and run the blocking operation through the runtime’s
established spawn_blocking pattern, or use the dispatcher’s supported mechanism
for offloading synchronous tools. Preserve the existing platform validation,
database error mapping, and response format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d37418d0-e97d-4cc5-8b8f-82c7c65984f1
📒 Files selected for processing (5)
src/channels.rssrc/cli/channel.rssrc/cli/mod.rssrc/main.rssrc/mcp_server.rs
…sport-error messages, and cap the outbound HTTP call with a timeout so a stalled platform can't hang the caller.
…ck + workflow-store smoke test) (#596) * feat: add dispatch preflight/init validation step (item #164) Duplicate-work detection (pre-claim): before dispatching an item, find_duplicate_pr searches GitHub for a PR already carrying the item's 'for item #N' marker. A merged match self-heals the item to completed and cleans up its worktree instead of letting a redispatch re-do already-merged work (the near-miss from items #122/#156, where tracked state fell out of sync with a merged PR). An open match skips dispatch and flags for human review instead of racing a second PR. Workflow-store smoke test: flare_workflow::smoke_test does a real save/delete/load round-trip against the actual on-disk SqliteStore schema. Wired into daemon boot (dashboard::server::run) and into dev_install::run via a new hidden 'daemon workflow-store-smoke-test' subcommand, run against the freshly built binary right after a binary swap -- the exact trigger (item #576) that let a broken delete_state go undetected for ~33h. On failure, refuses dispatch instead of letting every claimed item retry-and-fail silently into a stuck state. src/cli/work.rs is frozen at the LOC gate's 2100-line limit, so the new duplicate-PR logic and its tests live in their own included files (work_duplicate_pr.rs, work_duplicate_pr_tests.rs) rather than growing work.rs itself. Agentflare-Agent: claude-code Agentflare-Branch: task/164-dispatch-preflight-validation Agentflare-Item: 164 * fix review findings: duplicate-PR selection, claim/completion error handling, smoke-test ordering Addresses 6 CodeRabbit findings on PR #596, all independently verified against the actual code before fixing: - pick_duplicate_pr: a closed-but-unmerged PR was treated the same as a genuinely open one, permanently blocking a legitimate redispatch. Now filters to merged-or-open only. - handle_duplicate_pr: a failed/unconfirmed mark_completed was silently reported as success (comment posted, claim released, exit 0). Now returns a retryable failure and leaves the claim armed instead. - handle_duplicate_pr: item_release's result was discarded and claim_guard was disarmed unconditionally, so a failed release left nothing to retry it. Now only disarms on confirmed release success. - dashboard::server::run: engine().recover() ran before the workflow-store smoke test, so a broken store could still be touched by recovery before validation. Reordered so recovery is gated behind the smoke test too. - dev_install::verify_workflow_store: used a blocking Command::status() with no timeout, unlike its sibling verify_runs. Now uses the same spawn/try_wait/kill-with-deadline pattern. - github::pulls::find_by_item_marker: the search API call was unpaginated (GitHub's default page size is 30), so a matching PR beyond the first page would silently not be found. Now uses the existing get_paginated helper. Also fixes the schema-mismatch regression test in sqlite_store.rs: it put the intentional id/run_id mismatch on workflow_runs itself, so write_state failed on save and delete_state's own failure path was never actually exercised. Moved the mismatch to journal (write_state never touches it for an empty smoke-test state) and strengthened the assertion to confirm the failure comes from there. New tests: pick_duplicate_pr_ignores_a_closed_unmerged_pr, pick_duplicate_pr_selects_a_genuinely_open_pr_when_no_merged_match_exists. Verified: cargo build, cargo test --bin agentflare (1548/1548 passing), cargo test -p flare-workflow (all passing), cargo fmt --check, cargo clippy (CI's exact flags), scripts/loc-gate.sh. Agentflare-Agent: claude-code Agentflare-Branch: task/164-dispatch-preflight-validation Agentflare-Item: 164 * fix: gate work dispatch on successful pipeline registration/recovery engine().register_workflow()/recover() failures were only logged -- the worker pool and both supervisor ticks still started regardless, undermining the smoke-test gate's own fail-closed contract one level up: dispatch could start against pipeline state that's already known to be broken. Also fixes a compile error from the prior commit: recover() returns Result<Vec<WorkflowRunId>, WorkflowError>, not Result<(), _>. Agentflare-Agent: claude-code Agentflare-Branch: task/164-dispatch-preflight-validation Agentflare-Item: 164 --------- Co-authored-by: shiva <shiva@gosysinfo.tech>
find_duplicate_pr searches for any open PR carrying the item's "for item #N" marker, with no way to tell "a fresh dispatch about to redundantly open a second PR" apart from "a self-repair job reclaiming its own item's existing worktree/branch, whose entire job is to push a fix onto that exact PR." The latter hit the same short-circuit, bailed with "needs human review" without ever attempting a repair, and released the claim -- which only clears assignee_agent, never restores the state group, so the item was left orphaned in "started" with no label either run_discovery_tick or run_review_sweep would ever revisit (reproduced live on item #186/PR #597, whose CI stayed red with no further attempts). Exclude a still-open PR whose head branch matches the current worktree's branch from counting as a duplicate at all -- it's this job's own PR, not a competing one. A merged match still always short-circuits regardless of branch, since that's this check's other job: self-heal an item whose PR landed while its tracked state fell out of sync (items #122/#156). Agentflare-Agent: claude-code Agentflare-Branch: fix/dispatch-failure-ceiling-any-reason
Closes #152.
The outward half of the channels effort: send a plain-text message out to Telegram / Slack / Discord. A one-shot blocking POST that fits agentflare's sync/
ureqmodel, so an agent can push a message mid-run (MCP tool) or a human/script can from the CLI. The inbound daemon (getappz/flared#1) reuses this same path to send its replies.What's new
src/channels.rsPlatform { Telegram, Slack, Discord }withparse(case-insensitive) andsecret_name.build_request(platform, target, text, token)— the per-platform request shapes: TelegramPOST /bot{token}/sendMessage{chat_id,text}(token in URL); Slackchat.postMessage{channel,text}+Authorization: Bearer; DiscordPOST /channels/{id}/messages{content}+Authorization: Bot.interpret_response(platform, status, body)— status-only for Telegram/Discord; for Slack it inspects theokfield even on HTTP 200 and surfaces theerror.send()overureq(handles ureq's non-2xxErr(Status)arm) andsend_message(conn, …)which resolves the bot token from the encryptedgateway_secretsstore.agentflare channel send --to <platform> --target <id> <message>.channel_send { platform, target, message }so an agent can send during a turn.Design notes
<platform>_bot_token(AES-GCM inagentflare.db). Missing token → a clear error naming the secret.parse_mode/markdown are out of scope for this MVP.ureq, no new deps — matches agentflare's identity.Tests (TDD)
Platform::parsecase-insensitivity + unknown rejection;secret_namemapping.build_requestfor each platform (URL, auth scheme, body fields).interpret_response: Telegram/Discord status-only; Slackok:falseon HTTP 200 → error surfacing the reason.send_messagewith no configured token → error naming the secret.Summary by CodeRabbit
channel sendCLI command to deliver messages by platform and destination.