Skip to content

feat(telegram): approval forwarding + /sessions + /chat commands#33

Merged
wcatz merged 1 commit intomainfrom
feat/telegram-approvals
Mar 15, 2026
Merged

feat(telegram): approval forwarding + /sessions + /chat commands#33
wcatz merged 1 commit intomainfrom
feat/telegram-approvals

Conversation

@wcatz
Copy link
Copy Markdown
Owner

@wcatz wcatz commented Mar 15, 2026

Summary

  • Approval forwarding: When Ghost needs tool approval, sends to Telegram with Allow/Deny inline buttons
  • Deny with instructions: Reply to an approval message with text to deny and provide alternative instructions
  • /sessions: Lists all active Ghost sessions (ID, project, mode, message count)
  • /chat : Send a message to a specific Ghost session from Telegram
  • Short session IDs supported (prefix match)
  • Server ApprovalNotifier interface decouples approval forwarding from Telegram

Architecture

Ghost Agent Loop → approval_required → SSE to frontend
                                     → ApprovalNotifier.NotifyApproval() → Telegram
Telegram Allow button → POST /sessions/{id}/approve → Agent continues
Telegram reply text → POST /sessions/{id}/approve {instructions} → Agent gets feedback

Test plan

  • go vet ./... clean
  • go build ./cmd/ghost/ succeeds
  • No import cycles

Summary by CodeRabbit

Release Notes

  • New Features
    • Telegram bot integration for approvals: Receive approval requests with Approve/Deny buttons, including project and tool context for informed decisions.
    • Telegram session management: View active sessions with status indicators and send messages directly to specific sessions using bot commands.

- Tool approvals forwarded to Telegram with Allow/Deny inline buttons
- Reply to approval message with text to deny with instructions
- /sessions lists all active Ghost sessions with ID, project, mode
- /chat <session_id> <message> sends a message to a specific session
- Server ApprovalNotifier interface for external approval forwarding
- Callback query handlers for approve:/deny: button presses
- Short session IDs supported (prefix match)
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 15, 2026

📝 Walkthrough

Walkthrough

This PR integrates Telegram bot approval notifications into Ghost. It introduces an ApprovalNotifier interface, implements it in the Telegram bot to handle approval requests via inline buttons, and adds session management and chat commands to the bot for interactive server communication.

Changes

Cohort / File(s) Summary
Server Integration & Interface
cmd/ghost/main.go, internal/server/server.go
Defines ApprovalNotifier interface, introduces SetApprovalNotifier and ListenAddr methods on Server, and wires the Telegram bot as the approval notifier during server initialization with address exposure.
Approval Notification Trigger
internal/server/chat.go
Adds asynchronous call to approvalNotifier.NotifyApproval() when an approval is requested in handleSendMessage, forwarding session ID, project name, tool name, and input data.
Telegram Bot State & Commands
internal/telegram/bot.go
Extends Bot struct with serverAddr and approval state fields; registers "sessions" and "chat" commands; adds callback handlers for approve/deny buttons; integrates instruction-reply detection before default help response.
Telegram Approval Workflow
internal/telegram/approval.go
Implements NotifyApproval and SetServerAddr methods on Bot; handles inline Approve/Deny button callbacks with Ghost API integration; processes instruction-based denials; formats tool inputs for readable Telegram messages.
Telegram Session Management
internal/telegram/sessions.go
Introduces handleSessions command to list active sessions with status indicators and handleChat command to send messages to resolved sessions; includes fetchSessions and sendChatMessage helpers for Ghost API communication.

Sequence Diagram(s)

sequenceDiagram
    participant User as Telegram User
    participant Bot as Telegram Bot
    participant Server as Ghost Server
    participant DB as Database/State

    User->>Server: Trigger action requiring approval
    Server->>Bot: Call NotifyApproval(sessionID, projectName, toolName, input)
    Bot->>Bot: Store approval state (sessionID, toolName)
    Bot->>User: Send approval message with Approve/Deny buttons
    
    User->>Bot: Press Approve or Deny button
    Bot->>Bot: Parse callback (approve: or deny: prefix)
    Bot->>Server: POST /api/v1/sessions/{sessionID}/approve
    Server->>DB: Process approval/denial
    Server-->>Bot: Response (success/error)
    Bot->>User: Edit message to show approval result
    Bot->>Bot: Clear approval state

    alt User replies to approval message
        User->>Bot: Reply with instruction text
        Bot->>Bot: Check for pending approval
        Bot->>Server: POST /api/v1/sessions/{sessionID}/approve with instructions
        Server->>DB: Process denial with instructions
        Bot->>User: Confirm denial with instructions
        Bot->>Bot: Clear approval state
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through approval dreams,
Telegram buttons and Ghost API schemes,
Sessions resolved, denials blessed,
Our furry friend put the bot to the test!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: addition of approval forwarding to Telegram and two new Telegram commands (/sessions and /chat).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/telegram-approvals
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/ghost/main.go`:
- Around line 357-361: The bot is being given a bind address
(cfg.Server.ListenAddr) that may be non-dialable (e.g. ":8080", "0.0.0.0");
before calling tgBot.SetServerAddr, normalize cfg.Server.ListenAddr into a
dialable base URL used by the Telegram code (internal/telegram/approval.go and
internal/telegram/sessions.go). Implement logic near where tgBot.SetServerAddr
is called to parse cfg.Server.ListenAddr, ensure a host is present (replace
empty or wildcard hosts like ""/":"/ "0.0.0.0"/"::" with "127.0.0.1" or
"localhost"), ensure a port is present, prepend the http(s) scheme as
appropriate, and pass that normalized URL to tgBot.SetServerAddr (leaving
srv.SetApprovalNotifier(tgBot) unchanged).

In `@internal/telegram/approval.go`:
- Around line 17-22: The current single-slot approvalState (type approvalState
with fields mu, sessionID, toolName) allows a new approval to overwrite
tb.approval.sessionID and misroute instruction replies across sessions; change
the design to track approvals by the unique approval message identity (chat ID +
message ID) instead of a single global sessionID, store a map keyed by that pair
to approval metadata (sessionID, toolName), set the map entry when sending an
approval message, and update handleInstructionReply to look up the sessionID by
inspecting msg.ReplyToMessage (use ReplyToMessage.Chat.ID +
ReplyToMessage.MessageID as the key) and route replies to the correct session;
ensure proper locking around the map when creating, reading, and deleting
entries to avoid races.
- Around line 179-181: The callApproveAPI function currently uses http.Post
without a timeout and doesn't set an Authorization header, which can hang or
produce 401s; replace the http.Post calls in callApproveAPI with an
http.NewRequestWithContext (or create an http.Client with Timeout) using a
context with a deadline (e.g., 5–10s) and add the required Authorization header
(use the agent's stored credential field such as tb.apiKey/tb.authToken or a
provided config value) before sending via http.DefaultClient.Do (or the custom
client), and apply the same pattern to the other similar http calls flagged (the
nearby http.Post usages in callApproveAPI and the analogous patterns in
sessions.go).

In `@internal/telegram/sessions.go`:
- Around line 53-55: Create a small safe helper (e.g., shortID(id string)
string) that returns id if len(id) < 8 else id[:8], then replace raw slices
s.ID[:8] and fullID[:8] with shortID(s.ID) and shortID(fullID) respectively;
update the two locations referencing s.ID[:8] (the fmt.Fprintf call using
mdv2.Esc(s.ID[:8])) and the uses at the later block (lines ~105-106) so they
call mdv2.Esc(shortID(...)) to avoid panics on short IDs.
- Around line 98-105: The code calls tb.sendChatMessage(fullID, message) and
treats a nil error as success without inspecting the SSE stream payload; change
sendChatMessage (and its call sites around the replies at the shown block and
the similar block at 138-145) so it parses SSE events from the response body and
returns a non-nil error or special status when an SSE event with type "error" or
"approval_required" is received; update the calling site (where tb.reply
currently sends "📤 Sent to ...") to check that returned status/error and, on
SSE "error", reply with the error details (using mdv2.Esc(err.Error())) and
return, and on "approval_required" reply with a clear waiting/approval message
and return instead of reporting success.
- Around line 84-90: The current loop picks the first session whose s.ID has the
provided sessionID prefix (fullID variable), which can mis-route when multiple
IDs share that prefix; instead collect all matches from sessions where
strings.HasPrefix(s.ID, sessionID), then if zero matches return a not-found
error, if exactly one set fullID to that match, and if multiple return an
explicit ambiguity error (or prompt) listing the matching IDs so the caller can
be more specific; update the logic around the fullID variable and any callers
that expect a single resolved ID to handle the new ambiguous-case error path.
- Around line 109-113: fetchSessions (and the sibling POST call that creates
sessions) currently use http.Get/http.Post without timeouts or auth; replace
these with context-aware HTTP requests using a reusable http.Client with
Timeout: 10*time.Second, build requests via http.NewRequestWithContext(ctx,
...), and set the Authorization header to "Bearer "+tb.bearerToken (or
tb.config.BearerToken if you store it on the Bot struct). Concretely, in
Bot.fetchSessions use context.WithTimeout to create ctx, create req :=
http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("http://%s/api/v1/sessions",
tb.serverAddr), nil), set req.Header.Set("Authorization", "Bearer
"+tb.bearerToken), use client := &http.Client{Timeout: 10 * time.Second} and
client.Do(req) (similarly replace http.Post usage in the session-creation
function), and handle context/timeouts and non-2xx status codes accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9c3ed4df-30c5-49ec-8fac-b0bfc8939d5d

📥 Commits

Reviewing files that changed from the base of the PR and between 9a12efd and 93124b0.

📒 Files selected for processing (6)
  • cmd/ghost/main.go
  • internal/server/chat.go
  • internal/server/server.go
  • internal/telegram/approval.go
  • internal/telegram/bot.go
  • internal/telegram/sessions.go

Comment thread cmd/ghost/main.go
Comment on lines +357 to +361
// Wire Telegram bot as approval notifier + give it the server address.
if tgBot != nil {
srv.SetApprovalNotifier(tgBot)
tgBot.SetServerAddr(cfg.Server.ListenAddr)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's see the current state of cmd/ghost/main.go around lines 357-361
wc -l cmd/ghost/main.go

Repository: wcatz/ghost

Length of output: 76


🏁 Script executed:

# Read the specific lines mentioned in the review
sed -n '350,370p' cmd/ghost/main.go

Repository: wcatz/ghost

Length of output: 590


🏁 Script executed:

# Search for SetServerAddr to understand how it's used
rg "SetServerAddr" --type go -A 3 -B 3

Repository: wcatz/ghost

Length of output: 452


🏁 Script executed:

# Look for the ListenAddr configuration structure
rg "ListenAddr" --type go -A 2 -B 2

Repository: wcatz/ghost

Length of output: 2272


🏁 Script executed:

# Check for any URL building that uses the server address
rg "http://" --type go -A 2 -B 2 | head -50

Repository: wcatz/ghost

Length of output: 2537


🏁 Script executed:

head -30 cmd/ghost/main.go

Repository: wcatz/ghost

Length of output: 812


🏁 Script executed:

# Check if net is already imported
rg "^import" -A 20 cmd/ghost/main.go | head -30

Repository: wcatz/ghost

Length of output: 492


Normalize the server address before passing it to the Telegram bot.

cfg.Server.ListenAddr is a bind address and may be non-dialable (e.g., :8080, ::, 0.0.0.0). The bot uses this address to construct HTTP URLs in internal/telegram/approval.go and internal/telegram/sessions.go (e.g., http://<addr>/api/v1/sessions), which will fail if the address cannot be dialed from a client.

Suggested fix
+import "net"
@@
 	if tgBot != nil {
 		srv.SetApprovalNotifier(tgBot)
-		tgBot.SetServerAddr(cfg.Server.ListenAddr)
+		host, port, err := net.SplitHostPort(cfg.Server.ListenAddr)
+		if err == nil {
+			if host == "" || host == "0.0.0.0" || host == "::" {
+				host = "127.0.0.1"
+			}
+			tgBot.SetServerAddr(net.JoinHostPort(host, port))
+		} else {
+			tgBot.SetServerAddr(cfg.Server.ListenAddr)
+		}
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Wire Telegram bot as approval notifier + give it the server address.
if tgBot != nil {
srv.SetApprovalNotifier(tgBot)
tgBot.SetServerAddr(cfg.Server.ListenAddr)
}
// Wire Telegram bot as approval notifier + give it the server address.
if tgBot != nil {
srv.SetApprovalNotifier(tgBot)
host, port, err := net.SplitHostPort(cfg.Server.ListenAddr)
if err == nil {
if host == "" || host == "0.0.0.0" || host == "::" {
host = "127.0.0.1"
}
tgBot.SetServerAddr(net.JoinHostPort(host, port))
} else {
tgBot.SetServerAddr(cfg.Server.ListenAddr)
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/ghost/main.go` around lines 357 - 361, The bot is being given a bind
address (cfg.Server.ListenAddr) that may be non-dialable (e.g. ":8080",
"0.0.0.0"); before calling tgBot.SetServerAddr, normalize cfg.Server.ListenAddr
into a dialable base URL used by the Telegram code
(internal/telegram/approval.go and internal/telegram/sessions.go). Implement
logic near where tgBot.SetServerAddr is called to parse cfg.Server.ListenAddr,
ensure a host is present (replace empty or wildcard hosts like ""/":"/
"0.0.0.0"/"::" with "127.0.0.1" or "localhost"), ensure a port is present,
prepend the http(s) scheme as appropriate, and pass that normalized URL to
tgBot.SetServerAddr (leaving srv.SetApprovalNotifier(tgBot) unchanged).

Comment on lines +17 to +22
// approvalState tracks which session has a pending approval for instruction replies.
type approvalState struct {
mu sync.Mutex
sessionID string // session with pending approval
toolName string
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Global single-slot approval state can misroute replies across sessions.

A new approval overwrites tb.approval.sessionID, and instruction replies are accepted for any reply-to-bot message. With concurrent approvals, instructions can be sent to the wrong session.

Safer direction
 type approvalState struct {
 	mu        sync.Mutex
-	sessionID string // session with pending approval
-	toolName  string
+	pending map[string]string // key: "<chatID>:<messageID>" -> sessionID
 }

Track per approval message (chat/message pair), and in handleInstructionReply resolve sessionID using ReplyToMessage identity instead of a single global slot.

Also applies to: 56-59, 138-155, 169-172

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/telegram/approval.go` around lines 17 - 22, The current single-slot
approvalState (type approvalState with fields mu, sessionID, toolName) allows a
new approval to overwrite tb.approval.sessionID and misroute instruction replies
across sessions; change the design to track approvals by the unique approval
message identity (chat ID + message ID) instead of a single global sessionID,
store a map keyed by that pair to approval metadata (sessionID, toolName), set
the map entry when sending an approval message, and update
handleInstructionReply to look up the sessionID by inspecting msg.ReplyToMessage
(use ReplyToMessage.Chat.ID + ReplyToMessage.MessageID as the key) and route
replies to the correct session; ensure proper locking around the map when
creating, reading, and deleting entries to avoid races.

Comment on lines +179 to +181
if tb.serverAddr == "" {
return fmt.Errorf("server address not configured")
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n internal/telegram/approval.go | head -220

Repository: wcatz/ghost

Length of output: 7644


🏁 Script executed:

rg -n "http.Client|Transport|Authorization" internal/telegram/ -A 2 -B 2

Repository: wcatz/ghost

Length of output: 37


🏁 Script executed:

rg -n "type Bot struct" internal/telegram/ -A 20

Repository: wcatz/ghost

Length of output: 1350


🏁 Script executed:

rg -n "http\." internal/telegram/ -B 2 -A 2

Repository: wcatz/ghost

Length of output: 2223


callApproveAPI needs timeout and Authorization header handling.

The http.Post call on line 196 lacks a timeout, risking indefinite hangs if the server is unresponsive. Additionally, no Authorization header is set, causing 401 failures if the API requires authentication.

Use http.DefaultClient with a context timeout or create a custom client with http.Client{Timeout: ...}. Add an Authorization header to the request if the API requires it.

Also applies to: lines 195–197, 202–204, and similar patterns in sessions.go (lines 111, 132).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/telegram/approval.go` around lines 179 - 181, The callApproveAPI
function currently uses http.Post without a timeout and doesn't set an
Authorization header, which can hang or produce 401s; replace the http.Post
calls in callApproveAPI with an http.NewRequestWithContext (or create an
http.Client with Timeout) using a context with a deadline (e.g., 5–10s) and add
the required Authorization header (use the agent's stored credential field such
as tb.apiKey/tb.authToken or a provided config value) before sending via
http.DefaultClient.Do (or the custom client), and apply the same pattern to the
other similar http calls flagged (the nearby http.Post usages in callApproveAPI
and the analogous patterns in sessions.go).

Comment on lines +53 to +55
fmt.Fprintf(&sb, "%s `%s`\n %s \\| %s \\| %d msgs\n\n",
status, mdv2.Esc(s.ID[:8]), mdv2.Esc(s.ProjectName), mdv2.Esc(s.Mode), s.Messages)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Guard short-ID slicing to avoid panics.

Both s.ID[:8] and fullID[:8] can panic when IDs are shorter than 8 characters.

Safe short-ID helper
-		fmt.Fprintf(&sb, "%s `%s`\n  %s \\| %s \\| %d msgs\n\n",
-			status, mdv2.Esc(s.ID[:8]), mdv2.Esc(s.ProjectName), mdv2.Esc(s.Mode), s.Messages)
+		fmt.Fprintf(&sb, "%s `%s`\n  %s \\| %s \\| %d msgs\n\n",
+			status, mdv2.Esc(shortID(s.ID)), mdv2.Esc(s.ProjectName), mdv2.Esc(s.Mode), s.Messages)
@@
-	tb.reply(ctx, b, update, fmt.Sprintf("📤 Sent to `%s`:\n_%s_",
-		mdv2.Esc(fullID[:8]), mdv2.Esc(message)))
+	tb.reply(ctx, b, update, fmt.Sprintf("📤 Sent to `%s`:\n_%s_",
+		mdv2.Esc(shortID(fullID)), mdv2.Esc(message)))
+
+func shortID(id string) string {
+	if len(id) <= 8 {
+		return id
+	}
+	return id[:8]
+}

Also applies to: 105-106

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/telegram/sessions.go` around lines 53 - 55, Create a small safe
helper (e.g., shortID(id string) string) that returns id if len(id) < 8 else
id[:8], then replace raw slices s.ID[:8] and fullID[:8] with shortID(s.ID) and
shortID(fullID) respectively; update the two locations referencing s.ID[:8] (the
fmt.Fprintf call using mdv2.Esc(s.ID[:8])) and the uses at the later block
(lines ~105-106) so they call mdv2.Esc(shortID(...)) to avoid panics on short
IDs.

Comment on lines +84 to +90
fullID := ""
for _, s := range sessions {
if strings.HasPrefix(s.ID, sessionID) {
fullID = s.ID
break
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Handle ambiguous short ID prefixes explicitly.

Current logic takes the first prefix match, so /chat can target the wrong session when multiple IDs share the prefix.

Suggested resolution logic
-	fullID := ""
-	for _, s := range sessions {
-		if strings.HasPrefix(s.ID, sessionID) {
-			fullID = s.ID
-			break
-		}
-	}
-	if fullID == "" {
+	var matches []string
+	for _, s := range sessions {
+		if s.ID == sessionID {
+			matches = []string{s.ID}
+			break
+		}
+		if strings.HasPrefix(s.ID, sessionID) {
+			matches = append(matches, s.ID)
+		}
+	}
+	if len(matches) == 0 {
 		tb.reply(ctx, b, update, "Session not found\\. Use /sessions to list\\.")
 		return
 	}
+	if len(matches) > 1 {
+		tb.reply(ctx, b, update, "Session ID is ambiguous\\. Please use a longer prefix\\.")
+		return
+	}
+	fullID := matches[0]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fullID := ""
for _, s := range sessions {
if strings.HasPrefix(s.ID, sessionID) {
fullID = s.ID
break
}
}
var matches []string
for _, s := range sessions {
if s.ID == sessionID {
matches = []string{s.ID}
break
}
if strings.HasPrefix(s.ID, sessionID) {
matches = append(matches, s.ID)
}
}
if len(matches) == 0 {
tb.reply(ctx, b, update, "Session not found\\. Use /sessions to list\\.")
return
}
if len(matches) > 1 {
tb.reply(ctx, b, update, "Session ID is ambiguous\\. Please use a longer prefix\\.")
return
}
fullID := matches[0]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/telegram/sessions.go` around lines 84 - 90, The current loop picks
the first session whose s.ID has the provided sessionID prefix (fullID
variable), which can mis-route when multiple IDs share that prefix; instead
collect all matches from sessions where strings.HasPrefix(s.ID, sessionID), then
if zero matches return a not-found error, if exactly one set fullID to that
match, and if multiple return an explicit ambiguity error (or prompt) listing
the matching IDs so the caller can be more specific; update the logic around the
fullID variable and any callers that expect a single resolved ID to handle the
new ambiguous-case error path.

Comment on lines +98 to +105
// Send message via API. This returns an SSE stream — we just report success.
err = tb.sendChatMessage(fullID, message)
if err != nil {
tb.reply(ctx, b, update, "Error: "+mdv2.Esc(err.Error()))
return
}

tb.reply(ctx, b, update, fmt.Sprintf("📤 Sent to `%s`:\n_%s_",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don’t return success without interpreting SSE outcome events.

sendChatMessage drains the stream and returns nil on HTTP 200, but /api/v1/sessions/{id}/send reports runtime failures and approval waits inside SSE events (error, approval_required). This can show “sent” even when execution failed or is blocked.

Also applies to: 138-145

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/telegram/sessions.go` around lines 98 - 105, The code calls
tb.sendChatMessage(fullID, message) and treats a nil error as success without
inspecting the SSE stream payload; change sendChatMessage (and its call sites
around the replies at the shown block and the similar block at 138-145) so it
parses SSE events from the response body and returns a non-nil error or special
status when an SSE event with type "error" or "approval_required" is received;
update the calling site (where tb.reply currently sends "📤 Sent to ...") to
check that returned status/error and, on SSE "error", reply with the error
details (using mdv2.Esc(err.Error())) and return, and on "approval_required"
reply with a clear waiting/approval message and return instead of reporting
success.

Comment on lines +109 to +113
func (tb *Bot) fetchSessions() ([]apiSession, error) {
url := fmt.Sprintf("http://%s/api/v1/sessions", tb.serverAddr)
resp, err := http.Get(url) //nolint:gosec
if err != nil {
return nil, err
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n internal/telegram/sessions.go | head -150

Repository: wcatz/ghost

Length of output: 4842


🏁 Script executed:

rg -A 20 "type Bot struct" internal/telegram/

Repository: wcatz/ghost

Length of output: 1287


🏁 Script executed:

rg -i "bearer|authorization|auth" internal/telegram/sessions.go

Repository: wcatz/ghost

Length of output: 37


🏁 Script executed:

cd internal/telegram && go vet ./...

Repository: wcatz/ghost

Length of output: 2419


🏁 Script executed:

cd . && go vet ./internal/telegram/...

Repository: wcatz/ghost

Length of output: 37


🏁 Script executed:

rg -i "bearer|x-api-key|authorization" internal/

Repository: wcatz/ghost

Length of output: 1839


🏁 Script executed:

cat internal/config/config.example.yaml | grep -A 5 -B 5 "auth_token"

Repository: wcatz/ghost

Length of output: 402


🏁 Script executed:

rg -A 10 "authMiddleware" internal/server/server.go

Repository: wcatz/ghost

Length of output: 932


🏁 Script executed:

rg -B 20 "api/v1/sessions" internal/server/server.go | head -50

Repository: wcatz/ghost

Length of output: 715


🏁 Script executed:

rg -A 15 'r.Route\("/api/v1/sessions"' internal/server/server.go

Repository: wcatz/ghost

Length of output: 702


Add timeout and bearer token support to HTTP API calls.

http.Get (line 111) and http.Post (line 132) have no deadline and send no authorization header. These requests will fail with 401 when server auth is enabled, and without a timeout they can hang bot handlers indefinitely.

Create a context-aware http.Client with a reasonable timeout (e.g., 10 seconds) and add bearer token support from the bot's configuration.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/telegram/sessions.go` around lines 109 - 113, fetchSessions (and the
sibling POST call that creates sessions) currently use http.Get/http.Post
without timeouts or auth; replace these with context-aware HTTP requests using a
reusable http.Client with Timeout: 10*time.Second, build requests via
http.NewRequestWithContext(ctx, ...), and set the Authorization header to
"Bearer "+tb.bearerToken (or tb.config.BearerToken if you store it on the Bot
struct). Concretely, in Bot.fetchSessions use context.WithTimeout to create ctx,
create req := http.NewRequestWithContext(ctx, "GET",
fmt.Sprintf("http://%s/api/v1/sessions", tb.serverAddr), nil), set
req.Header.Set("Authorization", "Bearer "+tb.bearerToken), use client :=
&http.Client{Timeout: 10 * time.Second} and client.Do(req) (similarly replace
http.Post usage in the session-creation function), and handle context/timeouts
and non-2xx status codes accordingly.

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