diff --git a/cmd/odek/telegram.go b/cmd/odek/telegram.go index 64f122d5..f5dbe6e5 100644 --- a/cmd/odek/telegram.go +++ b/cmd/odek/telegram.go @@ -563,7 +563,8 @@ func telegramCmd(args []string) error { if err != nil || cs == nil { return "๐Ÿ“Š *Session Stats*\n\nNo active session yet. Send a message to start one.", nil } - return formatStats(cs), nil + used, limit := bot.DailyTokenUsage() + return formatStats(cs, used, limit), nil } // Handle /sessions โ€” list recent sessions belonging to this chat. @@ -2081,7 +2082,7 @@ func handleChatMessage( return } - reportError(bot, chatID, messageID, "Agent error: "+err.Error()) + reportError(bot, chatID, messageID, friendlyRunError(err)) return } @@ -2168,24 +2169,72 @@ func handleChatMessage( } // formatStats formats session statistics for the Telegram stats command. -func formatStats(cs *telegram.ChatSession) string { +func formatStats(cs *telegram.ChatSession, dailyUsed, dailyLimit int64) string { duration := time.Since(cs.CreatedAt).Truncate(time.Second) + budgetLine := fmt.Sprintf("Daily tokens: %s (unlimited)", formatThousands(dailyUsed)) + if dailyLimit > 0 { + pct := dailyUsed * 100 / dailyLimit + budgetLine = fmt.Sprintf("Daily tokens: %s / %s (%d%%)", + formatThousands(dailyUsed), formatThousands(dailyLimit), pct) + } + return fmt.Sprintf( "๐Ÿ“Š *Session Stats*\n\n"+ "Messages: %d\n"+ "Turns: %d\n"+ "Started: %s\n"+ "Duration: %s\n"+ - "Last active: %s", + "Last active: %s\n\n"+ + "%s", len(cs.Messages), cs.TurnCount, cs.CreatedAt.Format("Jan 02, 2006 15:04 UTC"), duration.String(), cs.LastActive.Format("15:04 UTC"), + budgetLine, ) } +// formatThousands groups a number with comma separators for readability. +func formatThousands(n int64) string { + s := strconv.FormatInt(n, 10) + start := 0 + if n < 0 { + start = 1 + } + var b strings.Builder + for i, c := range s { + if i > start && (len(s)-i)%3 == 0 { + b.WriteByte(',') + } + b.WriteRune(c) + } + return b.String() +} + +// friendlyRunError translates raw agent-run failures into actionable chat +// messages. Rate limits and timeouts are the two failure modes a phone user +// actually hits; both deserve an explanation and a next step, not a raw +// provider error string. +func friendlyRunError(err error) string { + var rle *llmclient.RateLimitError + if errors.As(err, &rle) { + msg := fmt.Sprintf("Agent error: rate-limited by the model provider after %d attempt(s). "+ + "Your session is intact and any completed work is saved. "+ + "Wait a moment and resend, or use /stats to check daily usage.", rle.Attempts) + if rle.RetryAfter > 0 { + msg += fmt.Sprintf(" Provider asked to retry in %s.", rle.RetryAfter.Truncate(time.Second)) + } + return msg + } + if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, os.ErrDeadlineExceeded) { + return "Agent error: the model request timed out. Your session is intact and any completed work is saved. "+ + "Resend your message to try again; if this keeps happening, check the provider status or raise the timeout in config." + } + return "Agent error: " + err.Error() +} + // โ”€โ”€ /plan_status (structured plan view) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ // maxTelegramPlanChars bounds the /plan_status reply. Telegram hard-caps one diff --git a/cmd/odek/telegram_test.go b/cmd/odek/telegram_test.go index a1a5aeac..e4df4c72 100644 --- a/cmd/odek/telegram_test.go +++ b/cmd/odek/telegram_test.go @@ -1126,7 +1126,7 @@ func TestFormatStats(t *testing.T) { CreatedAt: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC), LastActive: time.Date(2026, 1, 2, 3, 5, 5, 0, time.UTC), } - out := formatStats(cs) + out := formatStats(cs, 0, 0) if !strings.Contains(out, "Messages: 3") { t.Errorf("missing message count: %s", out) } diff --git a/cmd/odek/telegram_ux_ergonomics_test.go b/cmd/odek/telegram_ux_ergonomics_test.go new file mode 100644 index 00000000..ef3136c7 --- /dev/null +++ b/cmd/odek/telegram_ux_ergonomics_test.go @@ -0,0 +1,119 @@ +package main + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" + "time" + + "github.com/BackendStack21/odek/internal/llmclient" + "github.com/BackendStack21/odek/internal/telegram" +) + +// โ”€โ”€ /stats daily-budget surfacing โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// Gap: /stats showed message counts only; users had no visibility of daily +// token consumption against the configured budget. + +func TestFormatStats_ShowsDailyTokens(t *testing.T) { + cs := &telegram.ChatSession{ + ChatID: 1, + SessionID: "s1", + CreatedAt: time.Now().Add(-time.Hour), + LastActive: time.Now(), + TurnCount: 3, + } + out := formatStats(cs, 4200, 10000) + if !strings.Contains(out, "Daily tokens") { + t.Fatalf("stats should show daily token usage, got:\n%s", out) + } + if !strings.Contains(out, "4,200") || !strings.Contains(out, "10,000") { + t.Fatalf("stats should show used/limit numbers, got:\n%s", out) + } + if !strings.Contains(out, "42%") { + t.Fatalf("stats should show a percentage, got:\n%s", out) + } +} + +func TestFormatStats_UnlimitedBudget(t *testing.T) { + cs := &telegram.ChatSession{CreatedAt: time.Now(), LastActive: time.Now()} + out := formatStats(cs, 500, 0) + if !strings.Contains(out, "unlimited") { + t.Fatalf("stats with limit 0 should say unlimited, got:\n%s", out) + } +} + +// โ”€โ”€ Run-error ergonomics โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// Gap: provider failures surfaced raw error strings ("Agent error: ..."), +// giving rate-limited or timed-out users no guidance on what happened or +// what to do next. + +func TestFriendlyRunError_RateLimit(t *testing.T) { + err := &llmclient.RateLimitError{} + err.Attempts = 3 + out := friendlyRunError(err) + if !strings.Contains(out, "rate-limited") { + t.Fatalf("rate-limit error should be explained as rate-limited, got:\n%s", out) + } + if !strings.Contains(out, "3") { + t.Fatalf("rate-limit message should mention attempts, got:\n%s", out) + } + if !strings.Contains(out, "/stats") { + t.Fatalf("rate-limit message should point to /stats, got:\n%s", out) + } +} + +func TestFriendlyRunError_Timeout(t *testing.T) { + out := friendlyRunError(context.DeadlineExceeded) + if !strings.Contains(out, "timed out") { + t.Fatalf("deadline error should be explained as a timeout, got:\n%s", out) + } + if !strings.Contains(out, "session is intact") { + t.Fatalf("timeout message must reassure about session state, got:\n%s", out) + } +} + +func TestFriendlyRunError_Generic(t *testing.T) { + out := friendlyRunError(errors.New("boom")) + if !strings.Contains(out, "Agent error") || !strings.Contains(out, "boom") { + t.Fatalf("generic errors keep the raw message, got:\n%s", out) + } +} + +func TestFriendlyRunError_WrappedTimeout(t *testing.T) { + // Providers wrap deadlines in transport errors; the mapping must + // traverse the wrap chain, not just match the bare sentinel. + wrapped := fmt.Errorf("do request: %w", fmt.Errorf("post: %w", context.DeadlineExceeded)) + out := friendlyRunError(wrapped) + if !strings.Contains(out, "timed out") { + t.Fatalf("wrapped deadline should map to the timeout message, got:\n%s", out) + } +} + +func TestFormatThousands(t *testing.T) { + cases := []struct { + in int64 + want string + }{ + {0, "0"}, + {999, "999"}, + {1000, "1,000"}, + {4200, "4,200"}, + {1234567, "1,234,567"}, + {-1234, "-1,234"}, + } + for _, c := range cases { + if got := formatThousands(c.in); got != c.want { + t.Errorf("formatThousands(%d) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestFormatStats_OverBudget(t *testing.T) { + cs := &telegram.ChatSession{CreatedAt: time.Now(), LastActive: time.Now()} + out := formatStats(cs, 12000, 10000) + if !strings.Contains(out, "120%") { + t.Fatalf("over-budget usage should show >100%%, got:\n%s", out) + } +} diff --git a/docs/TELEGRAM.md b/docs/TELEGRAM.md index d43d4783..b2e9e4ea 100644 --- a/docs/TELEGRAM.md +++ b/docs/TELEGRAM.md @@ -220,6 +220,12 @@ so they must be approved per-call. After three approvals of the same class within 60 seconds, friction mode hides the Trust Session shortcut and adds a warning, breaking reflexive tap-through. +Every approval prompt states its deadline ("expires in 2m0s"). When the wait +window closes without a response, the prompt message is edited in place to an +**Expired** state with its inline buttons removed, so a stale keyboard can +never be tapped after the request is dead. An expired request is denied โ€” the +operation is not executed. + ### Outbound Media The agent can send files back to the chat either by emitting a `MEDIA:` prefix in its final answer (`MEDIA:photo:/path`, `MEDIA:voice:/path`, `MEDIA:document:/path`) or by calling `send_message` with the `file` parameter. Before any upload, the user must explicitly approve the operation, and the path is validated by `internal/telegram.ResolveMediaPathForChat`: @@ -249,7 +255,7 @@ defense-in-depth. | `/start` | Welcome message and bot introduction | | `/help` | Show all available commands with descriptions | | `/new` | Archive the current session and start a fresh conversation. Archived sessions are timestamped (`tg---`) and remain visible via `odek session list` | -| `/stats` | Show session statistics (turn count, model used, etc.) | +| `/stats` | Show session statistics (messages, turns, duration) plus daily token usage against the configured `daily_token_budget` | | `/jobs` | List background jobs for this chat | | `/stop` | Cancel a running agent task | diff --git a/internal/telegram/approver.go b/internal/telegram/approver.go index 039804b8..542b7754 100644 --- a/internal/telegram/approver.go +++ b/internal/telegram/approver.go @@ -17,7 +17,15 @@ import ( // approvalTimeout is how long the agent blocks waiting for a user response // via inline keyboard. If the user doesn't respond in time, the operation // is denied with a timeout error. -const approvalTimeout = 120 * time.Second +// approvalTimeout is how long an approval prompt waits for a response. +// A variable so tests can shrink it. On expiry the prompt message is +// visibly marked expired and its buttons removed. +var approvalTimeout = 120 * time.Second + +// approvalDeadlineText renders the human-readable deadline shown in prompts. +func approvalDeadlineText() string { + return approvalTimeout.Truncate(time.Second).String() +} // callbackDataPrefixes const ( @@ -274,6 +282,13 @@ func (a *TelegramApprover) PromptCommand(cls danger.RiskClass, cmd, description case <-a.cancel: return fmt.Errorf("approval cancelled: %s", cmd) case <-time.After(approvalTimeout): + // Mark the prompt visibly expired and strip the buttons so a stale + // keyboard can't be tapped after the wait window closed. + if err := a.bot.EditMessageText(a.ChatID, pr.messageID, + fmt.Sprintf("โฐ *Expired* โ€” no response within %s ยท the operation was not executed", approvalDeadlineText()), + &SendOpts{ParseMode: ParseModeMarkdownV2, ReplyMarkup: &InlineKeyboardMarkup{InlineKeyboard: [][]InlineKeyboardButton{}}}); err != nil { + a.log.Warn("telegram approver: expire prompt edit failed", "message_id", pr.messageID, "error", err) + } return fmt.Errorf("approval timeout: %s", cmd) } } @@ -358,12 +373,13 @@ func buildApprovalText(cls danger.RiskClass, cmd, description string) string { b.WriteString("Why: " + EscapeMarkdown(d) + "\n") } - // Reserve room for the fixed parts so the command body can be budgeted - // against Telegram's hard limit. The fences and a possible truncation - // marker are accounted for here. + // Reserve room for the fixed parts (including the expiry footer) so the + // command body can be budgeted against Telegram's hard limit. The fences + // and a possible truncation marker are accounted for here. const openFence = "```\n" const closeFence = "\n```" - overhead := b.Len() + len(openFence) + len(closeFence) + footer := fmt.Sprintf("\n\nโณ _This request expires in %s โ€” after that it is denied and nothing runs_", approvalDeadlineText()) + overhead := b.Len() + len(openFence) + len(closeFence) + len(footer) body := escapeCodeBlock(cmd) if budget := telegramMaxMsgLen - overhead; len(body) > budget { @@ -378,6 +394,7 @@ func buildApprovalText(cls danger.RiskClass, cmd, description string) string { b.WriteString(openFence) b.WriteString(body) b.WriteString(closeFence) + b.WriteString(footer) return b.String() } diff --git a/internal/telegram/approver_expiry_test.go b/internal/telegram/approver_expiry_test.go new file mode 100644 index 00000000..61006dae --- /dev/null +++ b/internal/telegram/approver_expiry_test.go @@ -0,0 +1,60 @@ +package telegram + +import ( + "strings" + "testing" + "time" + + "github.com/BackendStack21/odek/internal/danger" +) + +func dangerClassShell() danger.RiskClass { return danger.LocalWrite } + +// โ”€โ”€ Approval expiry UX โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// Gap: approval prompts sat with live buttons after the 120s wait expired, +// so stale keyboards invited taps that resolve nothing. The prompt must show +// its deadline up front, and the message must visibly expire on timeout. + +func TestBuildApprovalText_ShowsExpiryDeadline(t *testing.T) { + text := buildApprovalText(dangerClassShell(), "rm tmp/x", "") + if !strings.Contains(text, "expires in") { + t.Fatalf("approval text should state the deadline, got:\n%s", text) + } + if !strings.Contains(text, "2m") { + t.Fatalf("approval text should render the 120s deadline as 2m, got:\n%s", text) + } +} + +func TestPromptCommand_TimeoutExpiresPrompt(t *testing.T) { + rec := &requestRecorder{} + ts := testServer(t, rec) + defer ts.Close() + bot := testBot(t, ts) + + oldTimeout := approvalTimeout + approvalTimeout = 80 * time.Millisecond + defer func() { approvalTimeout = oldTimeout }() + + a := NewTelegramApprover(bot, 1, 0) + err := a.PromptCommand(dangerClassShell(), "echo hi", "") + if err == nil || !strings.Contains(err.Error(), "approval timeout") { + t.Fatalf("want approval timeout error, got %v", err) + } + + // The prompt message must be visibly expired: an editMessageText call + // must have removed the buttons and marked the request expired. + rec.mu.Lock() + defer rec.mu.Unlock() + found := false + for _, r := range rec.requests { + if strings.HasSuffix(r.Path, "/editMessageText") && + strings.Contains(r.Body, "Expired") && + strings.Contains(r.Body, "reply_markup") && + strings.Contains(r.Body, `"inline_keyboard":[]`) { + found = true + } + } + if !found { + t.Fatal("timeout should edit the approval message to an expired state") + } +}