fix bot safety and tgapi edge cases

AGENTS.md

@@ -0,0 +1,102 @@
+# AGENTS.md
+
+## Purpose
+This repository uses Codex for full-project Go code review, not diff-only review.
+
+When asked to review code, inspect the entire repository and use repository-wide context. Do not limit analysis to the latest commit, pull request diff, or recently changed files.
+
+## Review priorities
+Review the codebase with focus on:
+- correctness and reliability;
+- maintainability and architecture;
+- idiomatic Go;
+- testability;
+- performance where justified by code evidence;
+- security;
+- godoc quality.
+
+## Scope rules
+- Always review the whole repository unless the prompt explicitly narrows scope.
+- Check cross-package interactions, public APIs, package boundaries, and shared patterns.
+- Prefer concrete fixes over generic advice.
+- When feasible, make small, high-confidence improvements directly.
+- When uncertain, state confidence level and evidence.
+
+## Go review expectations
+Check for:
+- bugs, fragile logic, invalid assumptions, nil handling issues, resource leaks;
+- poor error handling;
+- misuse of context, cancellation, timeouts, retries, and cleanup;
+- race risks, deadlocks, blocking hazards, unsafe shared state;
+- non-idiomatic naming, APIs, interfaces, package structure, and error patterns;
+- unnecessary complexity, duplication, or weak abstractions;
+- obvious performance problems supported by the code;
+- security risks such as unsafe input handling, secret leakage, insecure logging, injection risks, and risky file or network operations.
+
+## Godoc rules
+Review comments for all declarations.
+
+### Exported declarations
+Exported types, funcs, methods, vars, and consts must have godoc comments.
+
+Each exported godoc comment must:
+- start with the identifier name;
+- explain the purpose or behavior;
+- be as short as possible without losing important meaning;
+- avoid repeating the signature mechanically;
+- stay high-signal and informative.
+
+### Unexported declarations
+Unexported types, funcs, methods, vars, and consts should generally not have godoc-style comments unless there is a strong reason.
+
+### Always report
+- missing godoc on exported declarations;
+- unnecessary godoc on unexported declarations;
+- comments that are too long, vague, redundant, or low-value;
+- comments that should be shortened or rewritten.
+
+When feasible, rewrite bad godoc into better versions.
+
+## Testing expectations
+Treat tests as a required part of review.
+
+- Assess existing test quality, not only test presence.
+- Add or propose as many useful tests as reasonably possible.
+- Prioritize public APIs, critical flows, edge cases, negative paths, boundary conditions, and concurrency-sensitive logic.
+- Prefer table-driven tests where appropriate.
+- Add regression tests for bugs you find.
+- If a case is hard to test directly, explain the gap and the best test strategy.
+
+## Commands
+Before finalizing changes, run the relevant project checks when available:
+- build
+- tests
+- lint
+- static analysis
+
+Prefer the repository’s documented commands. If multiple choices exist, use the most standard and least destructive ones first.
+
+## Output format
+For repo-wide review tasks, structure the result as:
+
+1. Overall summary
+2. Critical findings
+3. Major findings
+4. Minor findings
+5. Godoc issues
+6. Test gaps and added/proposed tests
+7. Good decisions worth keeping
+8. Summary of concrete changes made
+
+For each finding include:
+- location;
+- issue;
+- why it matters;
+- recommended fix.
+
+## Working style
+- Be direct, specific, and action-oriented.
+- Do not stop at style-only feedback.
+- Use full repository context before drawing conclusions.
+- Prefer minimal, high-confidence patches.
+- Preserve behavior unless intentionally fixing a bug.

bot.go

@@ -242,9 +242,10 @@ func (bot *Bot[T]) initLoggers(opts *BotOpts) {
 		path := fmt.Sprintf("%s/main.log", strings.TrimRight(opts.LoggerBasePath, "/"))
 		logger, err := utils.CreateFileLogger("BOT", level, path)
 		if err != nil {
-			bot.logger.Fatal(err)
+			bot.logger.Errorln(err)
+		} else {
+			bot.logger = logger
 		}
-		bot.logger = logger
 	}
 
 	if opts.UseRequestLogger {
@@ -253,9 +254,10 @@ func (bot *Bot[T]) initLoggers(opts *BotOpts) {
 			path := fmt.Sprintf("%s/requests.log", strings.TrimRight(opts.LoggerBasePath, "/"))
 			logger, err := utils.CreateFileLogger("REQUESTS", level, path)
 			if err != nil {
-				bot.logger.Fatal(err)
+				bot.logger.Errorln(err)
+			} else {
+				bot.RequestLogger = logger
 			}
-			bot.RequestLogger = logger
 		}
 	}
 }
@@ -275,7 +277,9 @@ func (bot *Bot[T]) SetUpdateOffset(offset int) {
 }
 
 // GetUpdateTypes returns the list of update types the bot is configured to receive.
-func (bot *Bot[T]) GetUpdateTypes() []tgapi.UpdateType { return bot.updateTypes }
+func (bot *Bot[T]) GetUpdateTypes() []tgapi.UpdateType {
+	return append([]tgapi.UpdateType(nil), bot.updateTypes...)
+}
 
 // GetLogger returns the main bot logger.
 func (bot *Bot[T]) GetLogger() *slog.Logger { return bot.logger }
@@ -383,12 +387,12 @@ func (bot *Bot[T]) Debug(debug bool) *Bot[T] {
 func (bot *Bot[T]) AddPlugins(plugin ...*Plugin[T]) *Bot[T] {
 	level := bot.GetLoggerLevel()
 	for _, p := range plugin {
-		if p.logger == nil {
-			logger := utils.CreateLogger(p.name, level)
-			p.SetLogger(logger)
+		cloned := clonePlugin(p)
+		if cloned.logger == nil {
+			cloned.logger = utils.CreateLogger(cloned.name, level)
 		}
-		bot.plugins = append(bot.plugins, *p)
-		bot.logger.Debugln(fmt.Sprintf("plugins with name \"%s\" registered", p.name))
+		bot.plugins = append(bot.plugins, cloned)
+		bot.logger.Debugln(fmt.Sprintf("plugins with name \"%s\" registered", cloned.name))
 	}
 	return bot
 }
@@ -535,12 +539,12 @@ func (bot *Bot[T]) AddDatabaseLoggerWriter(writer DbLogger[T]) *Bot[T] {
 //	_ = bot.Close(context.Background())
 func (bot *Bot[T]) RunWithContext(ctx context.Context) {
 	if len(bot.prefixes) == 0 {
-		bot.logger.Fatalln("no prefixes defined")
+		bot.logger.Errorln("no prefixes defined")
 		return
 	}
 
 	if len(bot.plugins) == 0 {
-		bot.logger.Fatalln("no plugins defined")
+		bot.logger.Errorln("no plugins defined")
 		return
 	}
 
@@ -604,3 +608,35 @@ func (bot *Bot[T]) RunWithContext(ctx context.Context) {
 func (bot *Bot[T]) Run() {
 	bot.RunWithContext(context.Background())
 }
+
+func clonePlugin[T DbContext](p *Plugin[T]) Plugin[T] {
+	cloned := Plugin[T]{
+		name:        p.name,
+		commands:    make(map[string]*Command[T], len(p.commands)),
+		payloads:    make(map[string]*Command[T], len(p.payloads)),
+		middlewares: append(extypes.Slice[Middleware[T]](nil), p.middlewares...),
+		skipAutoCmd: p.skipAutoCmd,
+		logger:      p.logger,
+		onClose:     p.onClose,
+	}
+
+	for name, command := range p.commands {
+		cloned.commands[name] = cloneCommand(command)
+	}
+	for name, command := range p.payloads {
+		cloned.payloads[name] = cloneCommand(command)
+	}
+
+	return cloned
+}
+
+func cloneCommand[T DbContext](command *Command[T]) *Command[T] {
+	if command == nil {
+		return nil
+	}
+
+	cloned := *command
+	cloned.args = append(extypes.Slice[CommandArg](nil), command.args...)
+	cloned.middlewares = append(extypes.Slice[Middleware[T]](nil), command.middlewares...)
+	return &cloned
+}

bot_test.go

@@ -0,0 +1,70 @@
+package laniakea
+
+import (
+	"path/filepath"
+	"reflect"
+	"testing"
+
+	"git.nix13.pw/scuroneko/laniakea/tgapi"
+	"git.nix13.pw/scuroneko/slog"
+)
+
+func TestGetUpdateTypesReturnsCopy(t *testing.T) {
+	bot := &Bot[NoDB]{updateTypes: []tgapi.UpdateType{tgapi.UpdateTypeMessage}}
+
+	got := bot.GetUpdateTypes()
+	got[0] = tgapi.UpdateTypeCallbackQuery
+
+	if want := []tgapi.UpdateType{tgapi.UpdateTypeMessage}; !reflect.DeepEqual(bot.updateTypes, want) {
+		t.Fatalf("GetUpdateTypes exposed internal slice: got %v want %v", bot.updateTypes, want)
+	}
+}
+
+func TestAddPluginsSnapshotsConfiguration(t *testing.T) {
+	bot := &Bot[NoDB]{logger: slog.CreateLogger()}
+	plugin := NewPlugin[NoDB]("demo")
+
+	cmd := plugin.NewCommand(func(ctx *MsgContext, db *NoDB) {}, "start")
+	plugin.AddMiddleware(*NewMiddleware("base", func(ctx *MsgContext, db *NoDB) bool { return true }))
+
+	bot.AddPlugins(plugin)
+
+	cmd.SetDescription("mutated after registration")
+	plugin.NewCommand(func(ctx *MsgContext, db *NoDB) {}, "late")
+	plugin.AddMiddleware(*NewMiddleware("late", func(ctx *MsgContext, db *NoDB) bool { return true }))
+
+	registered := bot.plugins[0]
+	if _, exists := registered.commands["late"]; exists {
+		t.Fatal("late command leaked into registered plugin snapshot")
+	}
+	if registered.commands["start"].description != "" {
+		t.Fatalf("registered command description unexpectedly mutated: %q", registered.commands["start"].description)
+	}
+	if len(registered.middlewares) != 1 {
+		t.Fatalf("registered middlewares unexpectedly mutated: got %d want 1", len(registered.middlewares))
+	}
+}
+
+func TestInitLoggersFallsBackToStdoutLoggerOnFileError(t *testing.T) {
+	bot := &Bot[NoDB]{}
+
+	bot.initLoggers(&BotOpts{
+		Debug:            true,
+		WriteToFile:      true,
+		UseRequestLogger: true,
+		LoggerBasePath:   filepath.Join(t.TempDir(), "missing", "nested"),
+	})
+
+	if bot.logger == nil {
+		t.Fatal("expected main logger fallback")
+	}
+	if bot.RequestLogger == nil {
+		t.Fatal("expected request logger fallback")
+	}
+	if err := bot.RequestLogger.Close(); err != nil {
+		t.Fatalf("failed to close request logger: %v", err)
+	}
+	if err := bot.logger.Close(); err != nil {
+		t.Fatalf("failed to close main logger: %v", err)
+	}
+}

drafts.go

@@ -38,12 +38,9 @@ func (g *LinearDraftIdGenerator) Next() uint64 {
 	return g.lastId.Add(1)
 }
 
-// DraftProvider manages a collection of Drafts and provides methods to create and
-// configure them. It holds shared configuration (chat, parse mode, entities) and
-// a draft ID generator.
+// DraftProvider manages a collection of Drafts and a shared draft ID generator.
 //
-// DraftProvider is NOT thread-safe. Concurrent access from multiple goroutines
-// requires external synchronization.
+// DraftProvider is safe for concurrent use.
 type DraftProvider struct {
 	mu        sync.RWMutex
 	api       *tgapi.API
@@ -133,10 +130,7 @@ type Draft struct {
 
 // NewDraft creates a new draft with the provided parse mode.
 //
-// The draft inherits the provider's chatID, messageThreadID, and entities.
-// If parseMode is zero, the provider's default parseMode is used.
-//
-// Panics if chatID is zero — call SetChat() on the provider first.
+// The caller must set a chat with SetChat before Push or Flush.
 func (p *DraftProvider) NewDraft(parseMode tgapi.ParseMode) *Draft {
 	id := p.generator.Next()
 	draft := &Draft{
@@ -224,6 +218,9 @@ func (d *Draft) Flush() error {
 	if d.Message == "" {
 		return nil
 	}
+	if d.chatID == 0 {
+		return ErrDraftChatIDZero
+	}
 
 	params := tgapi.SendMessageP{
 		ChatID:    d.chatID,

drafts_test.go

@@ -0,0 +1,36 @@
+package laniakea
+
+import (
+	"testing"
+
+	"git.nix13.pw/scuroneko/laniakea/tgapi"
+	"git.nix13.pw/scuroneko/slog"
+)
+
+func TestDraftFlushRequiresChatID(t *testing.T) {
+	draft := NewRandomDraftProvider(&tgapi.API{}).NewDraft(tgapi.ParseNone)
+	draft.Message = "hello"
+
+	if err := draft.Flush(); err != ErrDraftChatIDZero {
+		t.Fatalf("expected ErrDraftChatIDZero, got %v", err)
+	}
+}
+
+func TestMsgContextNewDraftWorksWithoutLimiter(t *testing.T) {
+	ctx := &MsgContext{
+		Api: &tgapi.API{},
+		Msg: &tgapi.Message{
+			Chat: &tgapi.Chat{ID: 42, Type: string(tgapi.ChatTypePrivate)},
+		},
+		Logger:        slog.CreateLogger(),
+		draftProvider: NewRandomDraftProvider(&tgapi.API{}),
+	}
+
+	draft := ctx.NewDraft()
+	if draft == nil {
+		t.Fatal("expected draft")
+	}
+	if draft.chatID != 42 {
+		t.Fatalf("unexpected chat id: %d", draft.chatID)
+	}
+}

msg_context.go

@@ -422,13 +422,23 @@ func (ctx *MsgContext) newDraft(parseMode tgapi.ParseMode) *Draft {
 		ctx.Logger.Errorln("can't create draft: ctx.Msg is nil")
 		return nil
 	}
-
-	c, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-	defer cancel()
-	if err := ctx.Api.Limiter.Wait(c, ctx.Msg.Chat.ID); err != nil {
-		ctx.Logger.Errorln(err)
+	if ctx.Api == nil {
+		ctx.Logger.Errorln("can't create draft: ctx.Api is nil")
 		return nil
 	}
+	if ctx.draftProvider == nil {
+		ctx.Logger.Errorln("can't create draft: ctx.draftProvider is nil")
+		return nil
+	}
+
+	if ctx.Api.Limiter != nil {
+		c, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+		if err := ctx.Api.Limiter.Wait(c, ctx.Msg.Chat.ID); err != nil {
+			ctx.Logger.Errorln(err)
+			return nil
+		}
+	}
 
 	draft := ctx.draftProvider.NewDraft(parseMode).SetChat(ctx.Msg.Chat.ID, ctx.Msg.MessageThreadID)
 	return draft

tgapi/methods_types.go

@@ -10,8 +10,8 @@ const (
 	ParseHTML ParseMode = "HTML"
 	// ParseMD enables legacy Markdown style parsing.
 	ParseMD ParseMode = "Markdown"
-	// ParseNone disables any parsing.
-	ParseNone ParseMode = "None"
+	// ParseNone disables parse_mode and leaves plain-text requests unannotated.
+	ParseNone ParseMode = ""
 )
 
 // EmptyParams is a placeholder for methods that take no parameters.

tgapi/parse_mode_test.go

@@ -0,0 +1,37 @@
+package tgapi
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+func TestParseNoneOmitsParseModeInJSON(t *testing.T) {
+	data, err := json.Marshal(SendMessageP{
+		ChatID:    42,
+		Text:      "hello",
+		ParseMode: ParseNone,
+	})
+	if err != nil {
+		t.Fatalf("Marshal returned error: %v", err)
+	}
+
+	if strings.Contains(string(data), `"parse_mode"`) {
+		t.Fatalf("expected parse_mode to be omitted, got %s", string(data))
+	}
+}
+
+func TestParseModeStillSerializesExplicitModes(t *testing.T) {
+	data, err := json.Marshal(SendMessageP{
+		ChatID:    42,
+		Text:      "hello",
+		ParseMode: ParseMDV2,
+	})
+	if err != nil {
+		t.Fatalf("Marshal returned error: %v", err)
+	}
+
+	if !strings.Contains(string(data), `"parse_mode":"MarkdownV2"`) {
+		t.Fatalf("expected MarkdownV2 parse_mode, got %s", string(data))
+	}
+}

tgapi/uploader_api.go

@@ -7,6 +7,7 @@ import (
 	"mime/multipart"
 	"net/http"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"git.nix13.pw/scuroneko/laniakea/utils"
@@ -241,7 +242,7 @@ func prepareMultipart[P any](files []UploaderFile, params P) (*bytes.Buffer, str
 // uploaderTypeByExt infers the Telegram upload field name from a file extension.
 // Falls back to UploaderDocumentType for unrecognized extensions.
 func uploaderTypeByExt(filename string) UploaderFileType {
-	ext := filepath.Ext(filename)
+	ext := strings.ToLower(filepath.Ext(filename))
 	switch ext {
 	case ".jpg", ".jpeg", ".png", ".webp", ".bmp":
 		return UploaderPhotoType

tgapi/uploader_api_test.go

@@ -104,6 +104,27 @@ func TestUploaderEncodesJSONFieldsAndLeavesAcceptEncodingToHTTPTransport(t *test
 	}
 }
 
+func TestNewUploaderFileDetectsFileTypeCaseInsensitively(t *testing.T) {
+	tests := []struct {
+		name     string
+		filename string
+		want     UploaderFileType
+	}{
+		{name: "uppercase photo", filename: "PHOTO.JPG", want: UploaderPhotoType},
+		{name: "uppercase voice", filename: "voice.OGG", want: UploaderVoiceType},
+		{name: "unknown defaults to document", filename: "archive.BIN", want: UploaderDocumentType},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			file := NewUploaderFile(tt.filename, []byte("x"))
+			if file.field != tt.want {
+				t.Fatalf("unexpected uploader field: got %q want %q", file.field, tt.want)
+			}
+		})
+	}
+}
+
 func readMultipartRequest(req *http.Request) (map[string]string, string, []byte, error) {
 	_, params, err := mime.ParseMediaType(req.Header.Get("Content-Type"))
 	if err != nil {

utils/limiter_test.go

@@ -0,0 +1,41 @@
+package utils
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+)
+
+func TestRateLimiterCheckDropOverflowHonorsGlobalLock(t *testing.T) {
+	rl := NewRateLimiter()
+	rl.SetGlobalLock(1)
+
+	if err := rl.Check(context.Background(), true, 0); !errors.Is(err, ErrDropOverflow) {
+		t.Fatalf("expected ErrDropOverflow, got %v", err)
+	}
+}
+
+func TestRateLimiterChatLocksAreScopedPerChat(t *testing.T) {
+	rl := NewRateLimiter()
+	rl.SetChatLock(42, 1)
+
+	if rl.Allow(42) {
+		t.Fatal("expected locked chat to be rejected")
+	}
+	if !rl.Allow(7) {
+		t.Fatal("expected unrelated chat to remain allowed")
+	}
+}
+
+func TestRateLimiterGlobalWaitRespectsContextCancellation(t *testing.T) {
+	rl := NewRateLimiter()
+	rl.SetGlobalLock(1)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)
+	defer cancel()
+
+	if err := rl.GlobalWait(ctx); !errors.Is(err, context.DeadlineExceeded) {
+		t.Fatalf("expected DeadlineExceeded, got %v", err)
+	}
+}