fix: user-facing command bug fixes and DB error handling by Divkix · Pull Request #650 · Divkix/Alita_Robot

Divkix · 2026-02-22T23:01:07Z

Summary

Systematic audit and fix of 13 bugs across user-facing command handlers in alita/modules/. Found via static analysis of all user-invocable functions.

Nil pointer dereference guards: logUsers, IdFromReply, misc.info, misc.echomsg, purge callbacks — all paths where ctx.EffectiveSender, msg.From, or GetSender() could be nil (channel messages)
Race condition fix: chatlist.txt replaced with os.CreateTemp for concurrent safety
Bounded retry: secureIntn crypto/rand loop capped at 10 iterations to prevent infinite spin
Hidden mutation removed: IsUserConnected() was mutating ctx.EffectiveChat as a side effect — callers now explicitly assign the returned connected chat
Blocking sleep removed: time.Sleep in removeBotKeyboard replaced with time.AfterFunc
XSS fix: custom incomplete HtmlEscape replaced with html.EscapeString
Goroutine panic recovery: fire-and-forget go db.ConnectId() wrapped with RecoverFromPanic
DB void-to-error refactor: 21 DB setter functions across 6 files converted from void returns to error returns, ~40 handler call sites updated to check errors and respond with localized messages
i18n: 2 new error message keys added to all 4 locale files (en, es, fr, hi)
Test updates: 6 DB test files updated to assert on new error-returning signatures

Files changed

32 source files, 36 total (including specs)
+2077/-213 lines

Test plan

go build ./... — clean
go vet ./... — clean
make test — passes (pre-existing tracing env var issue only)
make lint / golangci-lint — passes on all changed files
make check-translations — all translations present
Pre-commit hooks (golangci-lint, go fmt) passed on all 3 commits

…ovements - Fix logUsers nil sender panic on channel posts (US-001) - Fix purge handlers using ctx.EffectiveUser.Id instead of user.Id (US-002) - Fix info command nil sender guard (US-003) - Fix echomsg nil From guard (US-004) - Fix config.yml loading for alt_names resolution (US-006) - Fix IdFromReply nil sender guard (US-007) - Remove IsUserConnected context mutation, fix locks.go callers (US-008) - Add goroutine panic recovery for ConnectId call (US-009) - Replace HtmlEscape with html.EscapeString delegation (US-010) - Fix chatlist.txt race condition with os.CreateTemp (US-011) - Add bounded retry to secureIntn (US-012) - Replace time.Sleep with time.AfterFunc in removeBotKeyboard (US-013) - Add i18n keys: common_settings_save_failed, devs_chatlist_error

…ller error handling Convert 21 DB setter functions across 6 files from void returns to error returns, and update all ~40 call sites in handler modules to check errors and respond with localized error messages. Files changed: - antiflood_db.go: SetFlood, SetFloodMode, SetFloodMsgDel - greetings_db.go: 10 greeting setter functions - lang_db.go: ChangeUserLanguage, ChangeGroupLanguage - pin_db.go: SetAntiChannelPin, SetCleanLinked - warns_db.go: SetWarnLimit, SetWarnMode - filters_db.go: AddFilter, RemoveFilter

…gnatures Update 6 DB test files to match Batch 2 void-to-error refactor. Tests now assert on returned errors from setter functions that previously returned void. Also fix minor issue in devs.go.

Copilot

Pull request overview

This PR audits user-invocable command handlers and related DB setters to eliminate crash paths (nil dereferences), make settings writes error-aware, and address a handful of concurrency/robustness issues across alita/modules/ and alita/db/.

Changes:

Added nil-guards to prevent panics in high-traffic handlers/utilities (e.g., logUsers, reply sender extraction, misc handlers).
Refactored multiple DB “setter” functions from void returns to error returns and updated module call sites to surface failures via i18n.
Improved operational robustness (temporary file for /chatlist, bounded retry for crypto/rand, non-blocking keyboard removal, i18n loader update).

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
specs/check-user-commands-bugs/tasks.md	Adds implementation task breakdown and verification steps for the bug-fix audit.
specs/check-user-commands-bugs/research.md	Documents audit results and bug catalog across modules/handlers.
specs/check-user-commands-bugs/requirements.md	Captures acceptance criteria and edge cases for each bug fix.
specs/check-user-commands-bugs/design.md	Describes technical approach and cross-file refactors (notably DB error propagation).
locales/en.yml	Adds new generic settings-save failure key + chatlist error key.
locales/es.yml	Same i18n additions (Spanish).
locales/fr.yml	Same i18n additions (French).
locales/hi.yml	Same i18n additions (Hindi).
alita/i18n/loader.go	Stops skipping `config.yml` during locale loading (enables config-backed translator usage).
alita/utils/helpers/helpers.go	Replaces custom HTML escaping with `html.EscapeString` and removes `IsUserConnected` context mutation.
alita/utils/extraction/extraction.go	Adds nil-guard for reply sender extraction in `IdFromReply`.
alita/modules/users.go	Adds nil-guards for `EffectiveSender` and reply sender in the `logUsers` watcher.
alita/modules/misc.go	Adds nil-guard for `/tell` and `/info`; replaces blocking sleep with `time.AfterFunc`.
alita/modules/devs.go	Uses `os.CreateTemp` for `/chatlist` output to avoid filename races.
alita/modules/captcha.go	Bounds the crypto/rand retry loop to prevent infinite spinning.
alita/modules/helpers.go	Wraps a fire-and-forget goroutine with panic recovery.
alita/modules/locks.go	Updates some `IsUserConnected` callers to explicitly assign `ctx.EffectiveChat`.
alita/modules/greetings.go	Updates greetings handlers/callbacks to check DB write errors and reply with i18n failure text.
alita/modules/antiflood.go	Updates antiflood commands to check DB write errors and reply with i18n failure text.
alita/modules/language.go	Updates language callback handler to handle DB errors and answer the callback with failure text.
alita/modules/pins.go	Updates pin-related commands to handle DB errors and reply with i18n failure text.
alita/modules/warns.go	Updates warn settings commands to handle DB errors and reply with i18n failure text.
alita/modules/filters.go	Updates filter add/remove/overwrite flows to handle DB errors and surface generic failure text.
alita/modules/purges.go	Replaces `ctx.EffectiveUser.Id` usage with `user.Id` from `RequireUser` result.
alita/db/greetings_db.go	Refactors greetings DB setters to return `error`.
alita/db/antiflood_db.go	Refactors antiflood DB setters to return `error`.
alita/db/lang_db.go	Refactors language DB setters to return `error`.
alita/db/pin_db.go	Refactors pin DB setters to return `error`.
alita/db/warns_db.go	Refactors warns DB setters to return `error`.
alita/db/filters_db.go	Refactors filter DB setters to return `error`.
alita/db/greetings_db_test.go	Updates test call sites to compile with new signatures (but currently discards errors).
alita/db/antiflood_db_test.go	Updates test call sites to compile with new signatures.
alita/db/lang_db_test.go	Updates test call sites to compile with new signatures.
alita/db/pin_db_test.go	Updates test call sites to compile with new signatures (but currently discards errors).
alita/db/warns_db_test.go	Updates test call sites to compile with new signatures (but currently discards errors).
alita/db/filters_db_test.go	Updates test call sites to compile with new signatures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-22T23:11:03Z

alita/modules/users.go

+		replySender := repliedMsg.GetSender()
+		if replySender == nil {
+			return ext.ContinueGroups
+		}


In the reply-handling block, returning ext.ContinueGroups when replySender == nil exits logUsers early and skips the forwarded-origin update logic below. Instead, skip just the reply update when GetSender() is nil (e.g., wrap the reply update in if replySender != nil { ... }) and continue with the rest of the function.

Copilot · 2026-02-22T23:11:03Z

alita/modules/devs.go

+	openedFile, err := os.Open(tmpFile.Name())
+	if err != nil {
+		log.Error(err)
+		return err
+	}


This handler adds the devs_chatlist_error i18n key, but chatList currently returns raw errors on temp-file/open/write failures without sending a user-facing message, so the new translation key is unused. Either reply with the localized devs_chatlist_error (and return ext.EndGroups) on these error paths, or remove the unused key from locale files to keep translations tidy.

Copilot · 2026-02-22T23:11:03Z

alita/modules/locks.go

+	// connection status
+	chat := helpers.IsUserConnected(b, ctx, true, true)
+	if chat == nil {
 		return ext.EndGroups
 	}
-	chat := ctx.EffectiveChat
+	ctx.EffectiveChat = chat


After removing the side-effect mutation from helpers.IsUserConnected, this file still has callers (e.g., lockPerm / unlockPerm) that call IsUserConnected but then read ctx.EffectiveChat without assigning it from the returned chat. That will break connected-chat flows (notably in private via /connect). Update those callers to set ctx.EffectiveChat = connectedChat (or use the returned chat directly) after the nil check.

Copilot · 2026-02-22T23:11:03Z

alita/db/pin_db_test.go

 	errs := make(chan error, workers)

 	for i := range workers {
 		go func(i int) {
 			defer wg.Done()
 			pref := i%2 == 0
-			SetAntiChannelPin(chatID, pref)
-			SetCleanLinked(chatID, pref)
+			_ = SetAntiChannelPin(chatID, pref)
+			_ = SetCleanLinked(chatID, pref)
 		}(i)


In this concurrent test, the new error returns from SetAntiChannelPin / SetCleanLinked are ignored, so failures during concurrent writes won't be detected. Capture and assert these errors (e.g., send them to the existing errs channel) so the test actually validates the new error-returning behavior.

Copilot · 2026-02-22T23:11:04Z

alita/db/greetings_db_test.go

+	_ = SetWelcomeToggle(chatID, true)
 	settings := GetGreetingSettings(chatID)
 	if settings.WelcomeSettings == nil || !settings.WelcomeSettings.ShouldWelcome {
 		t.Fatalf("expected ShouldWelcome=true after SetWelcomeToggle(true)")
 	}

-	SetWelcomeToggle(chatID, false)
+	_ = SetWelcomeToggle(chatID, false)
 	settings = GetGreetingSettings(chatID)


These tests call the updated DB setters but discard the returned error (_ = SetWelcomeToggle(...), etc.). Since the main contract change is error propagation, the tests should assert err == nil so unexpected DB write failures are surfaced instead of silently ignored.

Copilot · 2026-02-22T23:11:04Z

alita/modules/users.go

+	if user == nil {
+		return ext.ContinueGroups
+	}


The early return on user == nil prevents the later reply-sender and forwarded-origin tracking from running. That breaks the documented edge cases where a channel post can still have a reply sender and/or msg.ForwardOrigin that should be processed. Instead of returning immediately, skip only the sender-specific update block when ctx.EffectiveSender is nil and continue to the reply/forward handling below.

Copilot · 2026-02-22T23:11:04Z

alita/modules/misc.go

+		if _, err := rMsg.Delete(b, nil); err != nil {
+			log.Error(err)
+		}


removeBotKeyboard now deletes the message in a timer callback, but it logs any delete error at Error level. The codebase already has helpers.DeleteMessageWithErrorHandling, which suppresses expected Telegram delete errors (e.g., message already deleted / can't be deleted) and logs them at Debug with context. Consider using that helper (or equivalent expected-error handling) inside the AfterFunc callback to avoid noisy logs in normal operation.

Suggested change

if _, err := rMsg.Delete(b, nil); err != nil {

log.Error(err)

}

helpers.DeleteMessageWithErrorHandling(b, rMsg.Chat.Id, rMsg.MessageId, "removeBotKeyboard")

Copilot · 2026-02-22T23:11:05Z

alita/modules/devs.go

@@ -111,16 +115,25 @@ func (moduleStruct) chatList(b *gotgbot.Bot, ctx *ext.Context) error {
 			fmt.Fprintf(&sb, "%d: %s\n", chatId, v.ChatName)
 		}
 	}
-	writeString += sb.String()

-	// If the file doesn't exist, create it or re-write it
-	err = os.WriteFile(fileName, []byte(writeString), 0o600)
+	_, err = tmpFile.WriteString(sb.String())
+	if err != nil {
+		log.Error(err)
+		return err
+	}


os.CreateTemp creates an open file descriptor; if WriteString (or later operations) return an error, the function returns without closing tmpFile. Add a defer tmpFile.Close() (and handle its error) immediately after successful CreateTemp to avoid leaking file descriptors on early returns.

Copilot · 2026-02-22T23:11:05Z

alita/db/warns_db_test.go

@@ -211,7 +211,7 @@ func TestSetWarnMode(t *testing.T) {
 		_ = DB.Where("chat_id = ?", chatID).Delete(&Chat{}).Error
 	})

-	SetWarnMode(chatID, "ban")
+	_ = SetWarnMode(chatID, "ban")



These DB setter functions now return error, but the test assigns the result to _ and doesn't assert success. To keep the test meaningful (and to catch unexpected DB failures), assert that the returned error is nil (e.g., fail the test if SetWarnLimit/SetWarnMode return an error).

- Fix file descriptor leak in devs.go: add defer tmpFile.Close() after os.CreateTemp, remove redundant explicit close - Fix logUsers early returns skipping forwarded-origin tracking: wrap sender-specific blocks in nil guards instead of returning early - Fix locks.go inconsistent ctx.EffectiveChat: add missing assignment from connectedChat in lockPerm and unlockPerm - Fix misc.go removeBotKeyboard: use DeleteMessageWithErrorHandling instead of raw Delete + log.Error to suppress expected Telegram errors - Fix test files discarding DB setter errors: replace _ = SetX(...) with proper error assertions in pin, greetings, and warns tests

Divkix added 4 commits February 22, 2026 15:36

chore(swarm): generate spec files for check-user-commands-bugs

b4d9595

fix(tests): update DB test assertions for error-returning function si…

8a5c306

…gnatures Update 6 DB test files to match Batch 2 void-to-error refactor. Tests now assert on returned errors from setter functions that previously returned void. Also fix minor issue in devs.go.

Copilot AI review requested due to automatic review settings February 22, 2026 23:01

Copilot started reviewing on behalf of Divkix February 22, 2026 23:01 View session

Merge branch 'main' into worktree-tender-seeking-hinton

a883a32

Divkix merged commit a135292 into main Feb 22, 2026
1 check passed

Divkix deleted the worktree-tender-seeking-hinton branch February 22, 2026 23:03

Copilot AI reviewed Feb 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: user-facing command bug fixes and DB error handling#650

fix: user-facing command bug fixes and DB error handling#650
Divkix merged 5 commits intomainfrom
worktree-tender-seeking-hinton

Divkix commented Feb 22, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Divkix commented Feb 22, 2026

Summary

Files changed

Test plan

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants