diff --git a/AGENTS.md b/AGENTS.md index 4b3b75a..c884dd5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -173,6 +173,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU - **Telegram outbound media hardening** (`internal/telegram/media_path.go` + `internal/telegram/approver.go` + `internal/telegram/handler.go` + `internal/tool/send_message.go` + `cmd/odek/telegram.go`) — paths supplied to `MEDIA:...` prefixes or `send_message(file=...)` are resolved to an absolute path and verified against an allowlist (cwd, `~/.odek/media/`, system temp dir). On Unix the final component is opened with `O_NOFOLLOW` and `fstat`'d to avoid a symlink TOCTOU race; `filepath.EvalSymlinks` ensures the resolved path does not escape the allowlist. Additionally, well-known secret subtrees (`~/.ssh`, `~/.aws`, `~/.gnupg`, `~/.odek` trust anchors, etc.) and any file whose basename starts with `.env` are rejected outright, and every outbound media upload now requires explicit user approval via `TelegramApprover.PromptMedia`, with an extra warning when the bot was launched from `$HOME` or `/`. - **Telegram plan file size cap** (`internal/telegram/plan.go`) — plan files larger than 1 MiB are rejected by `ReadPlan` and `MostRecentPlan`, and `ListPlans` only reads the first 8 KiB for preview. This prevents a prompt-injected agent from causing an OOM via a multi-hundred-megabyte plan file. - **Telegram log file permissions** (`internal/telegram/log.go`) — Telegram log files are created with `0600`, and `os.Chmod` hardens existing files. Chat IDs and task snippets are no longer world-readable by default. +- **Telegram chat-scoped sessions and plans** (`internal/telegram/session.go`, `internal/telegram/plan.go`, `cmd/odek/telegram.go`) — `/sessions`, `/resume`, `/prune`, `/plans`, `/plan_view`, `/plan_delete`, and `/plan_resume` are scoped to the requesting chat. Sessions are filtered by the `tg-` prefix, `ResumeSession` rejects cross-chat IDs, and plans live under `~/.odek/plans/chat/` so one chat cannot list, read, delete, or resume another chat's sessions/plans. - **Session ID entropy + session-scoped auth tokens** (`internal/session/session.go`, `cmd/odek/serve.go`) — session IDs now carry 128 bits of randomness (16 bytes / 32 hex chars); each session stores a 256-bit `AuthToken` required by `GET/DELETE/POST /api/sessions/`, `POST /api/cancel`, and WebSocket session-resume messages via `X-Session-Token` header, `session_token` cookie, or `auth_token` WS field. Per-IP rate limiting (60/min) on session lookups adds a brute-force backstop. - **Skill/episode untrusted wrapper** (`internal/loop/loop.go` + `odek.go`) — skill context and retrieved session-episode context are passed through the caller-provided untrusted wrapper (the same nonce'd `` boundary used for tool output) before being injected into the model's system context. This prevents a compromised or tainted skill/episode from being treated as trusted system instructions. - **`env` / `printenv` environment-dump gate** (`internal/danger/classifier.go`) — bare `env` and `printenv` invocations are classified as `system_write` because they can leak process-environment secrets that the redaction scanner does not recognise. `env VAR=value ` still classifies `` normally. diff --git a/cmd/odek/telegram.go b/cmd/odek/telegram.go index 01d0576..2fc5a42 100644 --- a/cmd/odek/telegram.go +++ b/cmd/odek/telegram.go @@ -385,9 +385,9 @@ func telegramCmd(args []string) error { return formatStats(cs), nil } - // Handle /sessions — list recent sessions from the store. + // Handle /sessions — list recent sessions belonging to this chat. if cmdName == "sessions" { - infos, err := sessionManager.ListSessions(10) + infos, err := sessionManager.ListSessions(chatID, 10) if err != nil { return fmt.Sprintf("❌ Failed to list sessions: %v", err), nil } @@ -411,7 +411,7 @@ func telegramCmd(args []string) error { return b.String(), nil } - // Handle /resume — switch to a different session. + // Handle /resume — switch to a different session owned by this chat. if cmdName == "resume" { sessionID := strings.TrimSpace(argsStr) if sessionID == "" { @@ -431,7 +431,7 @@ func telegramCmd(args []string) error { ), nil } - // Handle /prune [days] — clean up old sessions and plans. + // Handle /prune [days] — clean up old sessions and plans for this chat. if cmdName == "prune" { days := 30 if strings.TrimSpace(argsStr) != "" { @@ -441,11 +441,11 @@ func telegramCmd(args []string) error { return "❗ Usage: `/prune [days]`\n\nExample: `/prune 7` to remove sessions and plans older than 7 days.", nil } } - sessionsRemoved, err := sessionManager.PruneSessions(days) + sessionsRemoved, err := sessionManager.PruneSessions(chatID, days) if err != nil { return fmt.Sprintf("❌ Failed to prune sessions: %v", err), nil } - plansRemoved, err := sessionManager.PrunePlans(days) + plansRemoved, err := sessionManager.PrunePlans(chatID, days) if err != nil { return fmt.Sprintf("❌ Failed to prune plans: %v", err), nil } @@ -471,9 +471,10 @@ func telegramCmd(args []string) error { return "❗ Usage: `/plan `\n\nExample: `/plan Add user authentication with OAuth2`", nil } slug := telegram.Slugify(description) + planFile := fmt.Sprintf("~/.odek/plans/chat%d/%s.md", chatID, slug) prompt := fmt.Sprintf( "Create a detailed implementation plan for: %s\n\n"+ - "Save the plan as a markdown file to `~/.odek/plans/%s.md`. "+ + "Save the plan as a markdown file to `%s`. "+ "The plan should include:\n"+ "- Overview and goals\n"+ "- Architecture / design\n"+ @@ -481,16 +482,67 @@ func telegramCmd(args []string) error { "- File paths and key code locations\n"+ "- Testing strategy\n\n"+ "Use your write_file tool to save the plan.", - description, slug, + description, planFile, ) go handleChatMessage(chatID, messageID, userID, prompt, bot, handler, sessionManager, resolved, systemMessage, handlerLog) return fmt.Sprintf("📝 *Planning* `%s`…\n\n_Generating plan for: %s_", slug, description), nil } - // Handle /plan-resume — inject most recent plan into session context. + // Handle /plans — list saved plans for this chat. + if cmdName == "plans" { + infos, err := telegram.ListPlans(chatID, 20) + if err != nil { + return fmt.Sprintf("❌ Failed to list plans: %v", err), nil + } + if len(infos) == 0 { + return "📋 *Plans* — No plans found.\n\nCreate one with `/plan `", nil + } + var b strings.Builder + b.WriteString("📋 *Plans*\n\n") + for _, p := range infos { + ago := time.Since(p.ModTime).Round(time.Minute) + fmt.Fprintf(&b, "`%s` — %s ago\n", p.Slug, ago) + if p.Preview != "" { + fmt.Fprintf(&b, " _%s_\n", truncateStr(p.Preview, 60)) + } + } + b.WriteString("\nUse `/plan_view ` to read a plan.") + return b.String(), nil + } + + // Handle /plan_view — read a plan for this chat. + if cmdName == "plan_view" { + slug := strings.TrimSpace(argsStr) + if slug == "" { + return "❗ Usage: `/plan_view `\n\nUse `/plans` to see available plans.", nil + } + matched, content, err := telegram.ReadPlan(chatID, slug) + if err != nil { + return fmt.Sprintf("❌ %v", err), nil + } + if len(content) > 3900 { + content = content[:3900] + "\n\n… _(truncated — plan too long for Telegram)_" + } + return fmt.Sprintf("📄 *Plan: `%s`*\n\n%s", matched, content), nil + } + + // Handle /plan_delete — delete a plan for this chat. + if cmdName == "plan_delete" { + slug := strings.TrimSpace(argsStr) + if slug == "" { + return "❗ Usage: `/plan_delete `\n\nUse `/plans` to see available plans.", nil + } + matched, err := telegram.DeletePlan(chatID, slug) + if err != nil { + return fmt.Sprintf("❌ %v", err), nil + } + return fmt.Sprintf("🗑️ *Plan deleted*: `%s`", matched), nil + } + + // Handle /plan-resume — inject most recent plan for this chat into session context. if cmdName == "plan_resume" { - slug, content, err := telegram.MostRecentPlan() + slug, content, err := telegram.MostRecentPlan(chatID) if err != nil { return fmt.Sprintf("❌ %v", err), nil } @@ -2280,6 +2332,14 @@ func buttonsToMarkup(buttons [][]map[string]string) *telegram.InlineKeyboardMark return markup } +// truncateStr shortens s to maxLen, appending "…" if trimmed. +func truncateStr(s string, maxLen int) string { + if len(s) <= maxLen { + return s + } + return s[:maxLen] + "…" +} + // deleteToolTraceMessages deletes all individual tool trace messages for a chat. // Used to clean up after the agent finishes (success or error). func deleteToolTraceMessages(bot *telegram.Bot, chatID int64, msgIDs *sync.Map) { diff --git a/cmd/odek/telegram_test.go b/cmd/odek/telegram_test.go index c77ddf8..4d4b137 100644 --- a/cmd/odek/telegram_test.go +++ b/cmd/odek/telegram_test.go @@ -744,14 +744,6 @@ func narratorToolCallMessage(name, args string) string { } } -// truncateStr truncates a string for display in test logs. -func truncateStr(s string, n int) string { - if len(s) <= n { - return s - } - return s[:n] + "..." -} - // ── Tool Latency ──────────────────────────────────────────────────── // TestToolLatencyTracking verifies that recordToolStart and popToolLatency diff --git a/docs/SECURITY.md b/docs/SECURITY.md index cfaac6a..95118d4 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -543,6 +543,18 @@ Plan files live in `~/.odek/plans/` and are loaded by `/plan_view` and injected Telegram log files were created with world-readable `0644` permissions, exposing chat IDs and task snippets to other local users. `NewFileLogger` now creates log files with `0600` and `os.Chmod`'s existing files to the same mode. +### 39c. Telegram chat-scoped sessions and plans + +The Telegram bot's `/sessions`, `/resume`, `/prune`, `/plans`, `/plan_view`, `/plan_delete`, and `/plan_resume` commands previously operated on the global `~/.odek/sessions` and `~/.odek/plans` stores that are shared with the CLI. Any allowed chat could list the operator's CLI sessions (including task snippets that often contain secrets), resume one so its history entered the attacker's chat context, prune the operator's history, or read/delete plans created by other chats. + +These commands are now scoped to the requesting chat: + +- Each Telegram chat owns sessions with IDs of the form `tg-` (and timestamped archives `tg---`). `ListSessions`, `ResumeSession`, and `PruneSessions` only consider sessions whose ID starts with the caller's `tg-` prefix. +- `ResumeSession` explicitly rejects a direct ID that belongs to a different chat. +- Plans are stored under `~/.odek/plans/chat/`. `ListPlans`, `ReadPlan`, `DeletePlan`, and `MostRecentPlan` only look inside the caller's per-chat directory. Chat ID `0` is reserved as a global/admin scope mapping to the root `~/.odek/plans/` directory. + +This removes the cross-chat session/plan disclosure path while keeping the CLI and admin flows functional. + ### 40. `/api/resources` result limit cap The `/api/resources?q=...&limit=N` autocomplete endpoint previously accepted any positive `limit` value. It is now capped to 100 results both in the HTTP handler and in `Registry.Search`. This prevents a prompt-injected or attacker-forged request from forcing an unbounded directory walk and returning a multi-megabyte JSON response. diff --git a/docs/TELEGRAM.md b/docs/TELEGRAM.md index 4b59234..ce281ff 100644 --- a/docs/TELEGRAM.md +++ b/docs/TELEGRAM.md @@ -231,12 +231,12 @@ The agent can send files back to the chat either by emitting a `MEDIA:` prefix i | `/mode` | Show current agent modes (interaction_mode, sandbox, skills) | | `/restart` | Gracefully restart the bot process. Restricted to operator chats/users and rate-limited to once per 60 seconds. | | `/plan ` | Create a new plan from a natural language description | -| `/plans` | List all saved plans | -| `/plan-view ` | View a specific plan's content | -| `/plan-delete ` | Delete a saved plan | -| `/sessions` | List recent conversation sessions | -| `/resume ` | Resume a previous session by ID | -| `/prune [days]` | Clean up old sessions (default: 30 days) | +| `/plans` | List saved plans for this chat | +| `/plan-view ` | View a specific plan's content for this chat | +| `/plan-delete ` | Delete a saved plan for this chat | +| `/sessions` | List recent conversation sessions for this chat | +| `/resume ` | Resume a previous session owned by this chat | +| `/prune [days]` | Clean up old sessions and plans for this chat (default: 30 days) | | `/schedules` | List scheduled tasks (id, on/off, cron, next fire, last status) | | `/schedule ` | Manage scheduled tasks — `add`, `rm`, `enable`, `disable`, `run`, `next`, `view`. Mutating commands are restricted to configured operator chats/users. See [Managing schedules from Telegram](SCHEDULES.md#managing-from-telegram) | @@ -262,6 +262,7 @@ The `SessionManager` manages per-chat Telegram agent conversations, backed by th 5. **Session recall** — the user message is saved to the session store *before* the agent loop runs, enabling `session_search` to find the current conversation's data during the same turn 6. Active sessions survive bot restarts — on reconnect, the session is loaded from disk 7. **/now archives** — using `/new` archives the current session with a timestamped ID (`tg---`) before starting fresh. Archived sessions remain on disk and are visible via `odek session list`. +8. **Chat-scoped listing/resume/prune** — `/sessions`, `/resume`, and `/prune` only operate on sessions belonging to the requesting chat. A chat cannot list, resume, or delete another chat's sessions. ### Key Methods @@ -278,7 +279,7 @@ The `clarifyChannels` sync.Map provides per-chat channels for the agent to ask t ## Plan Management (`plan.go`) -Plans are stored as markdown files in `~/.odek/plans/.md`. Each plan is created from a natural language description and persisted for later review. +Plans are stored as markdown files in `~/.odek/plans/chat/.md`, isolating each Telegram chat's plans. Each plan is created from a natural language description and persisted for later review. The `/plan` command instructs the agent to save plans into the caller's per-chat directory, and `/plans`, `/plan_view`, `/plan_delete`, and `/plan_resume` only see plans belonging to that chat. ### Size Cap @@ -292,11 +293,11 @@ To prevent a prompt-injected agent from causing an out-of-memory condition, plan | Function | Purpose | |---|---| | `Slugify(description)` | Convert description to filesystem-safe slug (max 60 chars) | -| `ListPlans(limit)` | List plans sorted by modification time (newest first) | -| `ReadPlan(slug)` | Load plan content by exact slug or prefix match | -| `DeletePlan(slug)` | Delete plan by exact slug or prefix match | -| `MostRecentPlan()` | Return the most recently modified plan's content | -| `EnsurePlansDir()` | Create plans directory if it doesn't exist | +| `ListPlans(chatID, limit)` | List plans for a chat sorted by modification time (newest first) | +| `ReadPlan(chatID, slug)` | Load a chat's plan content by exact slug or prefix match | +| `DeletePlan(chatID, slug)` | Delete a chat's plan by exact slug or prefix match | +| `MostRecentPlan(chatID)` | Return the most recently modified plan's content for a chat | +| `EnsurePlansDir(chatID)` | Create per-chat plans directory if it doesn't exist | ### Slug Generation diff --git a/internal/telegram/chat_scope_test.go b/internal/telegram/chat_scope_test.go new file mode 100644 index 0000000..24a20fe --- /dev/null +++ b/internal/telegram/chat_scope_test.go @@ -0,0 +1,132 @@ +package telegram + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/BackendStack21/odek/internal/llm" + "github.com/BackendStack21/odek/internal/session" +) + +// Regression tests for M-2: Telegram session/plan commands must be scoped +// to the requesting chat. Cross-chat access must be rejected. + +func TestResumeSession_CrossChatRejected(t *testing.T) { + sm, _ := setupTestSessionManager(t) + + const ownerChat int64 = 999 + const attackerChat int64 = 100 + + if err := sm.Save(ownerChat, []llm.Message{{Role: "user", Content: "secret"}}); err != nil { + t.Fatalf("Save failed: %v", err) + } + + _, err := sm.ResumeSession(attackerChat, "tg-999") + if err == nil { + t.Fatal("ResumeSession should reject a session belonging to a different chat") + } + if !strings.Contains(err.Error(), "different chat") { + t.Errorf("error = %q, want 'different chat'", err) + } +} + +func TestListSessions_ChatScoped(t *testing.T) { + sm, _ := setupTestSessionManager(t) + + for _, chatID := range []int64{111, 222, 333} { + if err := sm.Save(chatID, []llm.Message{{Role: "user", Content: "msg"}}); err != nil { + t.Fatalf("Save(%d) failed: %v", chatID, err) + } + } + + infos, err := sm.ListSessions(222, 0) + if err != nil { + t.Fatalf("ListSessions failed: %v", err) + } + if len(infos) != 1 { + t.Errorf("ListSessions returned %d, want 1", len(infos)) + } + if len(infos) == 1 && infos[0].ID != "tg-222" { + t.Errorf("session ID = %q, want tg-222", infos[0].ID) + } +} + +func TestPruneSessions_ChatScoped(t *testing.T) { + sm, st := setupTestSessionManager(t) + + makeOldSession := func(chatID int64, id string) { + sess := &session.Session{ + ID: id, + CreatedAt: time.Now().Add(-60 * 24 * time.Hour), + UpdatedAt: time.Now().Add(-60 * 24 * time.Hour), + Task: id, + Messages: nil, + } + if err := st.Save(sess); err != nil { + t.Fatalf("store.Save(%q) failed: %v", id, err) + } + } + + makeOldSession(1, "tg-1-old") + makeOldSession(2, "tg-2-old") + + removed, err := sm.PruneSessions(1, 30) + if err != nil { + t.Fatalf("PruneSessions failed: %v", err) + } + if removed != 1 { + t.Errorf("PruneSessions removed %d, want 1", removed) + } + + // Chat 2's session must remain untouched. + _, err = st.Load("tg-2-old") + if err != nil { + t.Errorf("chat 2 session should not have been pruned: %v", err) + } +} + +func TestReadPlan_ChatScoped(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + + ownerDir := filepath.Join(tmp, ".odek", "plans", "chat111") + if err := os.MkdirAll(ownerDir, 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + os.WriteFile(filepath.Join(ownerDir, "secret.md"), []byte("secret plan"), 0644) + + _, _, err := ReadPlan(222, "secret") + if err == nil { + t.Fatal("ReadPlan should reject a plan belonging to a different chat") + } + if !strings.Contains(err.Error(), "no plan") && !strings.Contains(err.Error(), "no plans found") { + t.Errorf("error = %q, want plan-not-found", err) + } +} + +func TestDeletePlan_ChatScoped(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + + ownerDir := filepath.Join(tmp, ".odek", "plans", "chat111") + if err := os.MkdirAll(ownerDir, 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + os.WriteFile(filepath.Join(ownerDir, "secret.md"), []byte("secret plan"), 0644) + + _, err := DeletePlan(222, "secret") + if err == nil { + t.Fatal("DeletePlan should reject a plan belonging to a different chat") + } + if !strings.Contains(err.Error(), "no plan") && !strings.Contains(err.Error(), "no plans found") { + t.Errorf("error = %q, want plan-not-found", err) + } + + // Ensure the owner's file is still there. + if _, err := os.Stat(filepath.Join(ownerDir, "secret.md")); os.IsNotExist(err) { + t.Error("owner's plan file was deleted by cross-chat call") + } +} diff --git a/internal/telegram/commands.go b/internal/telegram/commands.go index 66b23e2..8b39510 100644 --- a/internal/telegram/commands.go +++ b/internal/telegram/commands.go @@ -3,7 +3,6 @@ package telegram import ( "fmt" "strings" - "time" ) // CommandDescriptor describes a slash command and its handler. @@ -183,55 +182,9 @@ func pruneHandler(args string) (string, error) { return "", nil } func planHandler(args string) (string, error) { return "", nil } -func plansHandler(args string) (string, error) { - infos, err := ListPlans(20) - if err != nil { - return fmt.Sprintf("❌ Failed to list plans: %v", err), nil - } - if len(infos) == 0 { - return "📋 *Plans* — No plans found.\n\nCreate one with `/plan `", nil - } - var b strings.Builder - b.WriteString("📋 *Plans*\n\n") - for _, p := range infos { - ago := time.Since(p.ModTime).Round(time.Minute) - fmt.Fprintf(&b, "`%s` — %s ago\n", p.Slug, ago) - if p.Preview != "" { - fmt.Fprintf(&b, " _%s_\n", truncateStr(p.Preview, 60)) - } - } - b.WriteString("\nUse `/plan_view ` to read a plan.") - return b.String(), nil -} - -func planViewHandler(args string) (string, error) { - slug := strings.TrimSpace(args) - if slug == "" { - return "❗ Usage: `/plan_view `\n\nUse `/plans` to see available plans.", nil - } - matched, content, err := ReadPlan(slug) - if err != nil { - return fmt.Sprintf("❌ %v", err), nil - } - // Telegram messages have a 4096 char limit. Truncate if needed. - if len(content) > 3900 { - content = content[:3900] + "\n\n… _(truncated — plan too long for Telegram)_" - } - return fmt.Sprintf("📄 *Plan: `%s`*\n\n%s", matched, content), nil -} - -func planDeleteHandler(args string) (string, error) { - slug := strings.TrimSpace(args) - if slug == "" { - return "❗ Usage: `/plan_delete `\n\nUse `/plans` to see available plans.", nil - } - matched, err := DeletePlan(slug) - if err != nil { - return fmt.Sprintf("❌ %v", err), nil - } - return fmt.Sprintf("🗑️ *Plan deleted*: `%s`", matched), nil -} - +func plansHandler(args string) (string, error) { return "", nil } +func planViewHandler(args string) (string, error) { return "", nil } +func planDeleteHandler(args string) (string, error) { return "", nil } func planResumeHandler(args string) (string, error) { return "", nil } // Schedule command handlers are intercepted in the bot's OnCommand callback diff --git a/internal/telegram/commands_test.go b/internal/telegram/commands_test.go index 3860b4e..d1ce9d5 100644 --- a/internal/telegram/commands_test.go +++ b/internal/telegram/commands_test.go @@ -1,8 +1,6 @@ package telegram import ( - "os" - "path/filepath" "strings" "testing" ) @@ -192,7 +190,7 @@ func TestAllHandlers_ReturnNoError(t *testing.T) { // handlers are stubs that return "". inlineOnly := map[string]bool{ "sessions": true, "resume": true, "prune": true, - "plan": true, "plan_resume": true, + "plan": true, "plans": true, "plan_view": true, "plan_delete": true, "plan_resume": true, "schedule": true, "schedules": true, } @@ -209,127 +207,6 @@ func TestAllHandlers_ReturnNoError(t *testing.T) { } } -// --------------------------------------------------------------------------- -// Plan command handler tests -// --------------------------------------------------------------------------- - -func TestPlansHandler_Empty(t *testing.T) { - tmp := t.TempDir() - t.Setenv("HOME", tmp) - - got, err := plansHandler("") - if err != nil { - t.Fatalf("plansHandler: %v", err) - } - if !strings.Contains(got, "No plans found") { - t.Errorf("expected 'No plans found', got: %s", got) - } -} - -func TestPlansHandler_WithPlans(t *testing.T) { - tmp := t.TempDir() - t.Setenv("HOME", tmp) - dir := filepath.Join(tmp, ".odek", "plans") - os.MkdirAll(dir, 0755) - os.WriteFile(filepath.Join(dir, "test-plan.md"), []byte("# Test Plan\n\nDo something."), 0644) - - got, err := plansHandler("") - if err != nil { - t.Fatalf("plansHandler: %v", err) - } - if !strings.Contains(got, "test-plan") { - t.Errorf("expected 'test-plan' in output, got: %s", got) - } - if !strings.Contains(got, "/plan_view") { - t.Errorf("expected '/plan_view' hint, got: %s", got) - } -} - -func TestPlanViewHandler_NoSlug(t *testing.T) { - got, err := planViewHandler("") - if err != nil { - t.Fatalf("planViewHandler: %v", err) - } - if !strings.Contains(got, "Usage") { - t.Errorf("expected usage message, got: %s", got) - } -} - -func TestPlanViewHandler_Found(t *testing.T) { - tmp := t.TempDir() - t.Setenv("HOME", tmp) - dir := filepath.Join(tmp, ".odek", "plans") - os.MkdirAll(dir, 0755) - os.WriteFile(filepath.Join(dir, "my-plan.md"), []byte("# My Plan\n\nStep 1. Step 2."), 0644) - - got, err := planViewHandler("my-plan") - if err != nil { - t.Fatalf("planViewHandler: %v", err) - } - if !strings.Contains(got, "My Plan") { - t.Errorf("expected plan content, got: %s", got) - } - if !strings.Contains(got, "Step 1") { - t.Errorf("expected 'Step 1' in content, got: %s", got) - } -} - -func TestPlanViewHandler_NotFound(t *testing.T) { - tmp := t.TempDir() - t.Setenv("HOME", tmp) - - got, err := planViewHandler("nonexistent") - if err != nil { - t.Fatalf("planViewHandler: %v", err) - } - if !strings.Contains(got, "❌") { - t.Errorf("expected error message, got: %s", got) - } -} - -func TestPlanDeleteHandler_NoSlug(t *testing.T) { - got, err := planDeleteHandler("") - if err != nil { - t.Fatalf("planDeleteHandler: %v", err) - } - if !strings.Contains(got, "Usage") { - t.Errorf("expected usage message, got: %s", got) - } -} - -func TestPlanDeleteHandler_Success(t *testing.T) { - tmp := t.TempDir() - t.Setenv("HOME", tmp) - dir := filepath.Join(tmp, ".odek", "plans") - os.MkdirAll(dir, 0755) - os.WriteFile(filepath.Join(dir, "delete-me.md"), []byte("bye"), 0644) - - got, err := planDeleteHandler("delete-me") - if err != nil { - t.Fatalf("planDeleteHandler: %v", err) - } - if !strings.Contains(got, "deleted") { - t.Errorf("expected 'deleted' message, got: %s", got) - } - // Verify file is gone. - if _, err := os.Stat(filepath.Join(dir, "delete-me.md")); !os.IsNotExist(err) { - t.Error("plan file still exists after delete") - } -} - -func TestPlanDeleteHandler_NotFound(t *testing.T) { - tmp := t.TempDir() - t.Setenv("HOME", tmp) - - got, err := planDeleteHandler("nonexistent") - if err != nil { - t.Fatalf("planDeleteHandler: %v", err) - } - if !strings.Contains(got, "❌") { - t.Errorf("expected error message, got: %s", got) - } -} - func TestPlanHandler_Stub(t *testing.T) { // planHandler is a stub — real plan creation is handled inline // by telegram.go's OnCommand which spawns an agent. diff --git a/internal/telegram/plan.go b/internal/telegram/plan.go index 44717d3..8192b13 100644 --- a/internal/telegram/plan.go +++ b/internal/telegram/plan.go @@ -39,8 +39,8 @@ type PlanInfo struct { Preview string // first line or ~80 chars of content } -// plansDir returns the plans directory path (~/.odek/plans/). -func plansDir() (string, error) { +// plansRoot returns the root plans directory path (~/.odek/plans/). +func plansRoot() (string, error) { home, err := os.UserHomeDir() if err != nil { return "", fmt.Errorf("plans: home dir: %w", err) @@ -48,9 +48,24 @@ func plansDir() (string, error) { return filepath.Join(home, ".odek", "plans"), nil } -// ensurePlansDir creates the plans directory if it doesn't exist. -func ensurePlansDir() (string, error) { - dir, err := plansDir() +// plansDirForChat returns the plans directory for chatID. ChatID 0 is a +// special global/admin scope that maps to the root plans directory; real +// Telegram chat IDs are non-zero, so production plans are always isolated +// under ~/.odek/plans/chat/. +func plansDirForChat(chatID int64) (string, error) { + root, err := plansRoot() + if err != nil { + return "", err + } + if chatID == 0 { + return root, nil + } + return filepath.Join(root, fmt.Sprintf("chat%d", chatID)), nil +} + +// ensurePlansDir creates the per-chat plans directory if it doesn't exist. +func ensurePlansDir(chatID int64) (string, error) { + dir, err := plansDirForChat(chatID) if err != nil { return "", err } @@ -131,9 +146,9 @@ func slugify(desc string) string { return slug } -// planPath returns the full path for a plan file given its slug. -func planPath(slug string) (string, error) { - dir, err := plansDir() +// planPath returns the full path for a plan file belonging to chatID. +func planPath(chatID int64, slug string) (string, error) { + dir, err := plansDirForChat(chatID) if err != nil { return "", err } @@ -142,11 +157,11 @@ func planPath(slug string) (string, error) { // ── CRUD ──────────────────────────────────────────────────────────────── -// ListPlans returns all .md plan files sorted by modification time +// ListPlans returns all .md plan files for chatID sorted by modification time // (newest first). If limit > 0, only the most recent `limit` plans are -// returned. Returns an empty slice if the plans directory doesn't exist. -func ListPlans(limit int) ([]PlanInfo, error) { - dir, err := plansDir() +// returned. Returns an empty slice if the chat's plans directory doesn't exist. +func ListPlans(chatID int64, limit int) ([]PlanInfo, error) { + dir, err := plansDirForChat(chatID) if err != nil { return nil, err } @@ -193,24 +208,24 @@ func ListPlans(limit int) ([]PlanInfo, error) { return infos, nil } -// ReadPlan loads a plan by slug prefix match. Returns the slug, content, -// and any error. If multiple plans match the prefix, the first exact match -// is preferred, then the first prefix match. Returns an error if no match -// is found. -func ReadPlan(slugPrefix string) (string, string, error) { +// ReadPlan loads a plan for chatID by slug prefix match. Returns the slug, +// content, and any error. If multiple plans match the prefix, the first exact +// match is preferred, then the first prefix match. Returns an error if no +// match is found. +func ReadPlan(chatID int64, slugPrefix string) (string, string, error) { slugPrefix = strings.ToLower(strings.TrimSpace(slugPrefix)) if slugPrefix == "" { return "", "", fmt.Errorf("plan slug required") } - dir, err := plansDir() + dir, err := plansDirForChat(chatID) if err != nil { return "", "", err } entries, err := os.ReadDir(dir) if os.IsNotExist(err) { - return "", "", fmt.Errorf("no plans directory found") + return "", "", fmt.Errorf("no plans found for this chat") } if err != nil { return "", "", fmt.Errorf("read plan: %w", err) @@ -250,22 +265,22 @@ func ReadPlan(slugPrefix string) (string, string, error) { return match, string(data), nil } -// DeletePlan removes a plan file by slug prefix match. Uses the same -// matching logic as ReadPlan. Returns the slug of the deleted plan. -func DeletePlan(slugPrefix string) (string, error) { +// DeletePlan removes a plan file for chatID by slug prefix match. Uses the +// same matching logic as ReadPlan. Returns the slug of the deleted plan. +func DeletePlan(chatID int64, slugPrefix string) (string, error) { slugPrefix = strings.ToLower(strings.TrimSpace(slugPrefix)) if slugPrefix == "" { return "", fmt.Errorf("plan slug required") } - dir, err := plansDir() + dir, err := plansDirForChat(chatID) if err != nil { return "", err } entries, err := os.ReadDir(dir) if os.IsNotExist(err) { - return "", fmt.Errorf("no plans directory found") + return "", fmt.Errorf("no plans found for this chat") } if err != nil { return "", fmt.Errorf("delete plan: %w", err) @@ -305,9 +320,9 @@ func DeletePlan(slugPrefix string) (string, error) { } // MostRecentPlan returns the slug and full content of the most recently -// modified plan file. Returns an error if no plans exist. -func MostRecentPlan() (string, string, error) { - infos, err := ListPlans(1) +// modified plan file for chatID. Returns an error if no plans exist. +func MostRecentPlan(chatID int64) (string, string, error) { + infos, err := ListPlans(chatID, 1) if err != nil { return "", "", err } diff --git a/internal/telegram/plan_test.go b/internal/telegram/plan_test.go index a96cd62..12f9b67 100644 --- a/internal/telegram/plan_test.go +++ b/internal/telegram/plan_test.go @@ -36,7 +36,7 @@ func TestEnsurePlansDir(t *testing.T) { tmp := t.TempDir() t.Setenv("HOME", tmp) - dir, err := ensurePlansDir() + dir, err := ensurePlansDir(0) if err != nil { t.Fatalf("ensurePlansDir: %v", err) } @@ -74,7 +74,7 @@ func TestListPlans(t *testing.T) { } // No limit. - infos, err := ListPlans(0) + infos, err := ListPlans(0, 0) if err != nil { t.Fatalf("ListPlans: %v", err) } @@ -90,9 +90,9 @@ func TestListPlans(t *testing.T) { } // With limit. - infos, _ = ListPlans(1) + infos, _ = ListPlans(0, 1) if len(infos) != 1 { - t.Fatalf("ListPlans(1) = %d items, want 1", len(infos)) + t.Fatalf("ListPlans(0, 1) = %d items, want 1", len(infos)) } } @@ -101,7 +101,7 @@ func TestListPlans_NoDir(t *testing.T) { t.Setenv("HOME", tmp) // Don't create .odek/plans. - infos, err := ListPlans(10) + infos, err := ListPlans(0, 10) if err != nil { t.Fatalf("ListPlans with no dir: %v", err) } @@ -118,7 +118,7 @@ func TestReadPlan_ExactMatch(t *testing.T) { os.MkdirAll(dir, 0755) os.WriteFile(filepath.Join(dir, "my-plan.md"), []byte("# My Plan\n\nDo things."), 0644) - slug, content, err := ReadPlan("my-plan") + slug, content, err := ReadPlan(0, "my-plan") if err != nil { t.Fatalf("ReadPlan: %v", err) } @@ -138,7 +138,7 @@ func TestReadPlan_PrefixMatch(t *testing.T) { os.MkdirAll(dir, 0755) os.WriteFile(filepath.Join(dir, "long-plan-name.md"), []byte("# Long Plan"), 0644) - slug, _, err := ReadPlan("long") + slug, _, err := ReadPlan(0, "long") if err != nil { t.Fatalf("ReadPlan prefix: %v", err) } @@ -156,7 +156,7 @@ func TestReadPlan_Ambiguous(t *testing.T) { os.WriteFile(filepath.Join(dir, "fix-login.md"), []byte("login"), 0644) os.WriteFile(filepath.Join(dir, "fix-logout.md"), []byte("logout"), 0644) - _, _, err := ReadPlan("fix") + _, _, err := ReadPlan(0, "fix") if err == nil { t.Fatal("expected ambiguous match error") } @@ -172,7 +172,7 @@ func TestReadPlan_NoMatch(t *testing.T) { dir := filepath.Join(tmp, ".odek", "plans") os.MkdirAll(dir, 0755) - _, _, err := ReadPlan("nonexistent") + _, _, err := ReadPlan(0, "nonexistent") if err == nil { t.Fatal("expected not found error") } @@ -190,7 +190,7 @@ func TestReadPlan_OversizeRejected(t *testing.T) { path := filepath.Join(dir, "huge-plan.md") os.WriteFile(path, []byte(strings.Repeat("x", maxPlanBytes+1)), 0644) - _, _, err := ReadPlan("huge-plan") + _, _, err := ReadPlan(0, "huge-plan") if err == nil { t.Fatal("expected error for oversized plan") } @@ -209,7 +209,7 @@ func TestReadPlan_AtSizeCapAllowed(t *testing.T) { content := strings.Repeat("x", maxPlanBytes) os.WriteFile(path, []byte(content), 0644) - slug, got, err := ReadPlan("max-plan") + slug, got, err := ReadPlan(0, "max-plan") if err != nil { t.Fatalf("ReadPlan at size cap: %v", err) } @@ -230,7 +230,7 @@ func TestMostRecentPlan_OversizeRejected(t *testing.T) { path := filepath.Join(dir, "huge-plan.md") os.WriteFile(path, []byte(strings.Repeat("x", maxPlanBytes+1)), 0644) - _, _, err := MostRecentPlan() + _, _, err := MostRecentPlan(0) if err == nil { t.Fatal("expected error for oversized most-recent plan") } @@ -248,7 +248,7 @@ func TestListPlans_OversizePreview(t *testing.T) { path := filepath.Join(dir, "huge-plan.md") os.WriteFile(path, []byte(strings.Repeat("x", maxPlanBytes+1)), 0644) - infos, err := ListPlans(0) + infos, err := ListPlans(0, 0) if err != nil { t.Fatalf("ListPlans: %v", err) } @@ -272,7 +272,7 @@ func TestDeletePlan(t *testing.T) { path := filepath.Join(dir, "delete-me.md") os.WriteFile(path, []byte("bye"), 0644) - slug, err := DeletePlan("delete-me") + slug, err := DeletePlan(0, "delete-me") if err != nil { t.Fatalf("DeletePlan: %v", err) } @@ -293,7 +293,7 @@ func TestDeletePlan_Ambiguous(t *testing.T) { os.WriteFile(filepath.Join(dir, "a-plan.md"), []byte("a"), 0644) os.WriteFile(filepath.Join(dir, "a-plan-2.md"), []byte("a2"), 0644) - _, err := DeletePlan("a") + _, err := DeletePlan(0, "a") if err == nil { t.Fatal("expected ambiguous match error") } @@ -315,7 +315,7 @@ func TestMostRecentPlan(t *testing.T) { os.WriteFile(filepath.Join(dir, "new.md"), []byte("# new plan\n\nContent."), 0644) os.Chtimes(filepath.Join(dir, "new.md"), now, now.Add(-1*time.Minute)) - slug, content, err := MostRecentPlan() + slug, content, err := MostRecentPlan(0) if err != nil { t.Fatalf("MostRecentPlan: %v", err) } @@ -331,7 +331,7 @@ func TestMostRecentPlan_Empty(t *testing.T) { tmp := t.TempDir() t.Setenv("HOME", tmp) - _, _, err := MostRecentPlan() + _, _, err := MostRecentPlan(0) if err == nil { t.Fatal("expected error for empty plans dir") } @@ -391,7 +391,7 @@ func TestPlanPath(t *testing.T) { tmp := t.TempDir() t.Setenv("HOME", tmp) - path, err := planPath("my-plan") + path, err := planPath(0, "my-plan") if err != nil { t.Fatalf("planPath error: %v", err) } @@ -409,7 +409,7 @@ func TestEnsurePlansDir_MkdirAllError(t *testing.T) { } t.Setenv("HOME", tmp) - _, err := ensurePlansDir() + _, err := ensurePlansDir(0) if err == nil { t.Fatal("expected error when MkdirAll fails") } @@ -425,14 +425,14 @@ func TestListPlans_ReadDirError(t *testing.T) { } t.Setenv("HOME", tmp) - _, err := ListPlans(0) + _, err := ListPlans(0, 0) if err == nil { t.Fatal("expected error when plans is not a directory") } } func TestReadPlan_EmptySlug(t *testing.T) { - _, _, err := ReadPlan("") + _, _, err := ReadPlan(0, "") if err == nil { t.Fatal("expected error for empty slug") } @@ -443,7 +443,7 @@ func TestReadPlan_NoPlansDir(t *testing.T) { t.Setenv("HOME", tmp) // No .odek/plans at all. - _, _, err := ReadPlan("anything") + _, _, err := ReadPlan(0, "anything") if err == nil { t.Fatal("expected error when no plans directory exists") } @@ -458,7 +458,7 @@ func TestReadPlan_ReadDirError(t *testing.T) { } t.Setenv("HOME", tmp) - _, _, err := ReadPlan("x") + _, _, err := ReadPlan(0, "x") if err == nil { t.Fatal("expected error when plans is not a directory") } @@ -475,14 +475,14 @@ func TestReadPlan_ReadFileError(t *testing.T) { // Delete the file so ReadFile fails. os.Remove(filepath.Join(dir, "exists.md")) - _, _, err := ReadPlan("exists") + _, _, err := ReadPlan(0, "exists") if err == nil { t.Fatal("expected error when file can't be read") } } func TestDeletePlan_EmptySlug(t *testing.T) { - _, err := DeletePlan("") + _, err := DeletePlan(0, "") if err == nil { t.Fatal("expected error for empty slug") } @@ -492,7 +492,7 @@ func TestDeletePlan_NoPlansDir(t *testing.T) { tmp := t.TempDir() t.Setenv("HOME", tmp) - _, err := DeletePlan("x") + _, err := DeletePlan(0, "x") if err == nil { t.Fatal("expected error when no plans directory") } @@ -505,7 +505,7 @@ func TestDeletePlan_ReadDirError(t *testing.T) { os.WriteFile(filepath.Join(odekDir, "plans"), []byte("x"), 0644) t.Setenv("HOME", tmp) - _, err := DeletePlan("x") + _, err := DeletePlan(0, "x") if err == nil { t.Fatal("expected error when plans is not a directory") } @@ -520,7 +520,7 @@ func TestDeletePlan_RemoveError(t *testing.T) { t.Setenv("HOME", tmp) // Try to delete — if we're root this may still succeed. - _, err := DeletePlan("locked") + _, err := DeletePlan(0, "locked") if err != nil { // Error is acceptable — test that it propagates. t.Logf("DeletePlan error (acceptable if running as root): %v", err) @@ -535,7 +535,7 @@ func TestMostRecentPlan_ReadFileError(t *testing.T) { os.Remove(filepath.Join(dir, "plan.md")) // remove after listing t.Setenv("HOME", tmp) - _, _, err := MostRecentPlan() + _, _, err := MostRecentPlan(0) // May or may not error depending on ListPlans caching behavior. // If it errors, verify it's a read error. if err != nil { @@ -552,7 +552,7 @@ func TestListPlans_SkipsNonMarkdown(t *testing.T) { os.MkdirAll(filepath.Join(dir, "subdir"), 0755) t.Setenv("HOME", tmp) - infos, err := ListPlans(0) + infos, err := ListPlans(0, 0) if err != nil { t.Fatalf("ListPlans error: %v", err) } @@ -573,7 +573,7 @@ func TestReadPlan_MultiplePrefixMatches(t *testing.T) { os.WriteFile(filepath.Join(dir, "fix-db.md"), []byte("db"), 0644) t.Setenv("HOME", tmp) - _, _, err := ReadPlan("fix") + _, _, err := ReadPlan(0, "fix") if err == nil { t.Fatal("expected error for ambiguous prefix with >2 matches") } @@ -589,7 +589,7 @@ func TestDeletePlan_NoMatchFound(t *testing.T) { os.WriteFile(filepath.Join(dir, "some-plan.md"), []byte("x"), 0644) t.Setenv("HOME", tmp) - _, err := DeletePlan("nonexistent") + _, err := DeletePlan(0, "nonexistent") if err == nil { t.Fatal("expected error for non-matching slug") } diff --git a/internal/telegram/session.go b/internal/telegram/session.go index f0cb90d..7512c9b 100644 --- a/internal/telegram/session.go +++ b/internal/telegram/session.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "strings" "sync" "time" @@ -298,46 +299,69 @@ type SessionInfo struct { Turns int // number of user turns } -// ListSessions returns metadata for all sessions in the backing store, -// sorted by most-recent-first, limited to `limit` entries. If limit <= 0, -// all sessions are returned. -func (sm *SessionManager) ListSessions(limit int) ([]SessionInfo, error) { - sessions, err := sm.Store.List(limit) +// ListSessions returns metadata for sessions belonging to chatID, sorted by +// most-recent-first and limited to `limit` entries. If limit <= 0, all +// matching sessions are returned. +func (sm *SessionManager) ListSessions(chatID int64, limit int) ([]SessionInfo, error) { + all, err := sm.Store.List(0) if err != nil { return nil, fmt.Errorf("list sessions: %w", err) } - infos := make([]SessionInfo, len(sessions)) - for i, s := range sessions { - infos[i] = SessionInfo{ + + prefix := fmt.Sprintf("tg-%d", chatID) + var infos []SessionInfo + for _, s := range all { + if !strings.HasPrefix(s.ID, prefix) { + continue + } + infos = append(infos, SessionInfo{ ID: s.ID, Task: s.Task, CreatedAt: s.CreatedAt, UpdatedAt: s.UpdatedAt, Turns: s.Turns, - } + }) + } + + // Sort newest first. + sort.Slice(infos, func(i, j int) bool { + return infos[i].UpdatedAt.After(infos[j].UpdatedAt) + }) + + if limit > 0 && len(infos) > limit { + infos = infos[:limit] } return infos, nil } -// ResumeSession loads a session from the backing store and binds it to -// the given chatID. This replaces any existing session for that chat. -// sessionID can be a partial prefix match — the first matching session -// (by ID prefix or task contains) is used. Returns the new ChatSession -// or an error if no matching session is found. +// ResumeSession loads a session belonging to chatID from the backing store +// and binds it to that chat. This replaces any existing session for that +// chat. sessionID can be a partial prefix match — the first matching session +// (by ID prefix or task contains) among this chat's sessions is used. +// Returns an error if the matched session belongs to a different chat. func (sm *SessionManager) ResumeSession(chatID int64, sessionID string) (*ChatSession, error) { if sessionID == "" { return nil, fmt.Errorf("session ID required — use /sessions to list") } + prefix := fmt.Sprintf("tg-%d", chatID) + // Try direct load first. sess, err := sm.Store.Load(sessionID) + if err == nil && sess != nil && !strings.HasPrefix(sess.ID, prefix) { + return nil, fmt.Errorf("session %q belongs to a different chat", sess.ID) + } + if err != nil || sess == nil { - // Prefix match: search all sessions for a match. + // Prefix match: search only this chat's sessions. all, listErr := sm.Store.List(0) if listErr != nil { return nil, fmt.Errorf("list sessions: %w", listErr) } for i, s := range all { + if !strings.HasPrefix(s.ID, prefix) { + continue + } if strings.HasPrefix(s.ID, sessionID) || strings.Contains(strings.ToLower(s.Task), strings.ToLower(sessionID)) { sess = &all[i] @@ -367,30 +391,47 @@ func (sm *SessionManager) ResumeSession(chatID int64, sessionID string) (*ChatSe return cs, nil } -// PruneSessions deletes sessions that haven't been updated in `days` days -// or more. Returns the number of sessions removed. -func (sm *SessionManager) PruneSessions(days int) (int, error) { +// PruneSessions deletes sessions belonging to chatID that haven't been +// updated in `days` days or more. Returns the number of sessions removed. +func (sm *SessionManager) PruneSessions(chatID int64, days int) (int, error) { if days <= 0 { days = 30 } before := time.Now().Add(-time.Duration(days) * 24 * time.Hour) - return sm.Store.Cleanup(before) + prefix := fmt.Sprintf("tg-%d", chatID) + + all, err := sm.Store.List(0) + if err != nil { + return 0, fmt.Errorf("list sessions: %w", err) + } + + removed := 0 + for _, s := range all { + if !strings.HasPrefix(s.ID, prefix) { + continue + } + if s.UpdatedAt.Before(before) { + if err := sm.Store.Delete(s.ID); err != nil { + return removed, fmt.Errorf("delete session %s: %w", s.ID, err) + } + removed++ + } + } + return removed, nil } -// PrunePlans deletes plan files (~/.odek/plans/*.md) older than `days` days. -// Plans don't have a formal store yet — this scans the directory and checks -// file modification times. Returns the number of plan files removed. If the -// plans directory doesn't exist, returns 0, nil. -func (sm *SessionManager) PrunePlans(days int) (int, error) { +// PrunePlans deletes plan files for chatID (~/.odek/plans/chat/*.md) +// older than `days` days. Returns the number of plan files removed. If the +// chat's plans directory doesn't exist, returns 0, nil. +func (sm *SessionManager) PrunePlans(chatID int64, days int) (int, error) { if days <= 0 { days = 30 } - home, err := os.UserHomeDir() + dir, err := plansDirForChat(chatID) if err != nil { return 0, nil } - plansDir := filepath.Join(home, ".odek", "plans") - entries, err := os.ReadDir(plansDir) + entries, err := os.ReadDir(dir) if os.IsNotExist(err) { return 0, nil } @@ -409,7 +450,7 @@ func (sm *SessionManager) PrunePlans(days int) (int, error) { continue } if info.ModTime().Before(before) { - path := filepath.Join(plansDir, e.Name()) + path := filepath.Join(dir, e.Name()) if err := os.Remove(path); err != nil && !os.IsNotExist(err) { return removed, fmt.Errorf("prune plans: remove %q: %w", e.Name(), err) } diff --git a/internal/telegram/session_concurrent_test.go b/internal/telegram/session_concurrent_test.go index 322bd05..9dbbf86 100644 --- a/internal/telegram/session_concurrent_test.go +++ b/internal/telegram/session_concurrent_test.go @@ -1,6 +1,7 @@ package telegram import ( + "fmt" "sync" "testing" "time" @@ -150,15 +151,17 @@ func TestSave_RaceFreeLoadAfterSave(t *testing.T) { // this captures the address of the reused loop variable, not the element. func TestResumeSession_LoopVariableBug(t *testing.T) { sm, store := setupTestSessionManager(t) + const chatID int64 = 42 + prefix := fmt.Sprintf("tg-%d", chatID) - // Create multiple sessions with known IDs. + // Create multiple sessions belonging to the same chat. for _, s := range []struct { id string task string }{ - {"sess-alpha", "Fix login page"}, - {"sess-beta", "Implement API rate limiting"}, - {"sess-gamma", "Refactor database layer"}, + {prefix + "-alpha", "Fix login page"}, + {prefix + "-beta", "Implement API rate limiting"}, + {prefix + "-gamma", "Refactor database layer"}, } { if err := store.Save(&session.Session{ ID: s.id, @@ -172,15 +175,15 @@ func TestResumeSession_LoopVariableBug(t *testing.T) { } // Resume by session ID prefix — should find the matching session. - cs, err := sm.ResumeSession(42, "sess-beta") + cs, err := sm.ResumeSession(chatID, prefix+"-beta") if err != nil { t.Fatalf("ResumeSession failed: %v", err) } if cs == nil { t.Fatal("ResumeSession returned nil") } - if cs.SessionID != "sess-beta" { - t.Errorf("SessionID = %q, want %q", cs.SessionID, "sess-beta") + if cs.SessionID != prefix+"-beta" { + t.Errorf("SessionID = %q, want %q", cs.SessionID, prefix+"-beta") } if cs.TurnCount != 0 { t.Errorf("TurnCount = %d, want 0", cs.TurnCount) diff --git a/internal/telegram/session_test.go b/internal/telegram/session_test.go index e5cda24..d55785d 100644 --- a/internal/telegram/session_test.go +++ b/internal/telegram/session_test.go @@ -715,12 +715,29 @@ func TestCacheMissAfterDelete(t *testing.T) { // --------------------------------------------------------------------------- func TestListSessions(t *testing.T) { - sm, _ := setupTestSessionManager(t) - for i := int64(1); i <= 3; i++ { - sm.Save(i, []llm.Message{{Role: "user", Content: fmt.Sprintf("msg %d", i)}}) //nolint:errcheck + sm, st := setupTestSessionManager(t) + const chatID int64 = 42 + + // Current session for the chat. + if err := sm.Save(chatID, []llm.Message{{Role: "user", Content: "current"}}); err != nil { + t.Fatalf("Save failed: %v", err) + } + // Plus a couple of archived sessions for the same chat. + for _, suffix := range []string{"alpha", "beta"} { + id := fmt.Sprintf("tg-%d-%s", chatID, suffix) + sess := &session.Session{ + ID: id, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + Task: id, + Messages: nil, + } + if err := st.Save(sess); err != nil { + t.Fatalf("store.Save(%q) failed: %v", id, err) + } } - infos, err := sm.ListSessions(0) + infos, err := sm.ListSessions(chatID, 0) if err != nil { t.Fatalf("ListSessions failed: %v", err) } @@ -730,17 +747,29 @@ func TestListSessions(t *testing.T) { } func TestListSessions_Limited(t *testing.T) { - sm, _ := setupTestSessionManager(t) - for i := int64(1); i <= 5; i++ { - sm.Save(i, []llm.Message{{Role: "user", Content: fmt.Sprintf("msg %d", i)}}) //nolint:errcheck + sm, st := setupTestSessionManager(t) + const chatID int64 = 42 + + for i := 0; i < 5; i++ { + id := fmt.Sprintf("tg-%d-archive-%d", chatID, i) + sess := &session.Session{ + ID: id, + CreatedAt: time.Now(), + UpdatedAt: time.Now().Add(-time.Duration(i) * time.Minute), + Task: id, + Messages: nil, + } + if err := st.Save(sess); err != nil { + t.Fatalf("store.Save(%q) failed: %v", id, err) + } } - infos, err := sm.ListSessions(3) + infos, err := sm.ListSessions(chatID, 3) if err != nil { - t.Fatalf("ListSessions(3) failed: %v", err) + t.Fatalf("ListSessions(%d, 3) failed: %v", chatID, err) } - if len(infos) > 3 { - t.Errorf("ListSessions(3) returned %d, want <= 3", len(infos)) + if len(infos) != 3 { + t.Errorf("ListSessions(%d, 3) returned %d, want 3", chatID, len(infos)) } } @@ -750,8 +779,9 @@ func TestListSessions_Limited(t *testing.T) { func TestResumeSession_DirectID(t *testing.T) { sm, _ := setupTestSessionManager(t) + const chatID int64 = 999 - err := sm.Save(999, []llm.Message{ + err := sm.Save(chatID, []llm.Message{ {Role: "user", Content: "resume test"}, {Role: "assistant", Content: "resume response"}, }) @@ -759,15 +789,15 @@ func TestResumeSession_DirectID(t *testing.T) { t.Fatalf("Save failed: %v", err) } - cs, err := sm.ResumeSession(100, "tg-999") + cs, err := sm.ResumeSession(chatID, fmt.Sprintf("tg-%d", chatID)) if err != nil { t.Fatalf("ResumeSession failed: %v", err) } - if cs.ChatID != 100 { - t.Errorf("ChatID = %d, want 100", cs.ChatID) + if cs.ChatID != chatID { + t.Errorf("ChatID = %d, want %d", cs.ChatID, chatID) } - if cs.SessionID != "tg-999" { - t.Errorf("SessionID = %q, want tg-999", cs.SessionID) + if cs.SessionID != fmt.Sprintf("tg-%d", chatID) { + t.Errorf("SessionID = %q, want tg-%d", cs.SessionID, chatID) } if len(cs.Messages) != 2 { t.Errorf("Messages length = %d, want 2", len(cs.Messages)) @@ -776,9 +806,9 @@ func TestResumeSession_DirectID(t *testing.T) { t.Errorf("Messages[0] = %q, want %q", cs.Messages[0].Content, "resume test") } - cached, ok := sm.Cache[100] + cached, ok := sm.Cache[chatID] if !ok { - t.Fatal("expected chat 100 to be cached after ResumeSession") + t.Fatalf("expected chat %d to be cached after ResumeSession", chatID) } if cached != cs { t.Errorf("cached pointer differs") @@ -803,18 +833,19 @@ func TestResumeSession_NotFound(t *testing.T) { func TestPruneSessions(t *testing.T) { sm, _ := setupTestSessionManager(t) + const chatID int64 = 1 - err := sm.Save(1, []llm.Message{{Role: "user", Content: "keep"}}) + err := sm.Save(chatID, []llm.Message{{Role: "user", Content: "keep"}}) if err != nil { t.Fatalf("Save failed: %v", err) } - removed, err := sm.PruneSessions(0) + removed, err := sm.PruneSessions(chatID, 0) if err != nil { t.Fatalf("PruneSessions failed: %v", err) } if removed != 0 { - t.Errorf("PruneSessions(0) removed %d, want 0 (no old sessions)", removed) + t.Errorf("PruneSessions(%d, 0) removed %d, want 0 (no old sessions)", chatID, removed) } } @@ -824,11 +855,12 @@ func TestPruneSessions(t *testing.T) { func TestPrunePlans(t *testing.T) { sm, _ := setupTestSessionManager(t) + const chatID int64 = 1 - // Create a plans directory with temp files. + // Create a per-chat plans directory with temp files. home := t.TempDir() t.Setenv("HOME", home) - plansDir := filepath.Join(home, ".odek", "plans") + plansDir := filepath.Join(home, ".odek", "plans", fmt.Sprintf("chat%d", chatID)) if err := os.MkdirAll(plansDir, 0755); err != nil { t.Fatalf("MkdirAll plans: %v", err) } @@ -846,12 +878,12 @@ func TestPrunePlans(t *testing.T) { } // Prune with 30 days — should only remove old.md. - removed, err := sm.PrunePlans(30) + removed, err := sm.PrunePlans(chatID, 30) if err != nil { t.Fatalf("PrunePlans failed: %v", err) } if removed != 1 { - t.Errorf("PrunePlans(30) = %d, want 1", removed) + t.Errorf("PrunePlans(%d, 30) = %d, want 1", chatID, removed) } // old.md should be gone, recent.md should remain. @@ -865,12 +897,13 @@ func TestPrunePlans(t *testing.T) { func TestPrunePlans_NoDir(t *testing.T) { sm, _ := setupTestSessionManager(t) + const chatID int64 = 1 // Set HOME to a temp dir with no plans subdirectory. home := t.TempDir() t.Setenv("HOME", home) - removed, err := sm.PrunePlans(30) + removed, err := sm.PrunePlans(chatID, 30) if err != nil { t.Fatalf("PrunePlans failed: %v", err) }