fix: improve error handling, add godoc, and fix bugs across core packages

- pkg/pid: log os.Remove errors instead of ignoring, fix Windows syscall
- pkg/session: add godoc to all exported methods, handle os.MkdirAll error
- pkg/logger: fix nil err in Errorf, handle logFile.Close errors, add godoc
- pkg/cron: add input validation in AddJob, add godoc to all exported methods
- pkg/config: log SaveConfig errors during migration, add godoc

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

pkg/config/config.go

@@ -340,6 +340,7 @@ type WhatsAppConfig struct {
 	UseNative          bool                `json:"use_native"           yaml:"-" env:"PICOCLAW_CHANNELS_WHATSAPP_USE_NATIVE"`
 	SessionStorePath   string              `json:"session_store_path"   yaml:"-" env:"PICOCLAW_CHANNELS_WHATSAPP_SESSION_STORE_PATH"`
 	AllowFrom          FlexibleStringSlice `json:"allow_from"           yaml:"-" env:"PICOCLAW_CHANNELS_WHATSAPP_ALLOW_FROM"`
+	GroupTrigger       GroupTriggerConfig  `json:"group_trigger,omitempty" yaml:"-"`
 	ReasoningChannelID string              `json:"reasoning_channel_id" yaml:"-" env:"PICOCLAW_CHANNELS_WHATSAPP_REASONING_CHANNEL_ID"`
 }
 
@@ -1027,7 +1028,9 @@ func LoadConfig(path string) (*Config, error) {
 			return nil, fmt.Errorf("failed to load existing security config: %w", secErr)
 		}
 		defer func(cfg *Config) {
-			_ = SaveConfig(path, cfg)
+			if saveErr := SaveConfig(path, cfg); saveErr != nil {
+				logger.ErrorF("failed to save config after v0 migration", map[string]any{"error": saveErr})
+			}
 		}(cfg)
 	case 1:
 		// V1→V2 migration: infer Enabled and migrate channel config fields
@@ -1061,7 +1064,9 @@ func LoadConfig(path string) (*Config, error) {
 		}
 
 		defer func(cfg *Config) {
-			_ = SaveConfig(path, cfg)
+			if saveErr := SaveConfig(path, cfg); saveErr != nil {
+				logger.ErrorF("failed to save config after v1 migration", map[string]any{"error": saveErr})
+			}
 		}(cfg)
 		logger.InfoF(
 			"config migrate success",

pkg/config/gateway.go

@@ -33,6 +33,7 @@ func canonicalGatewayLogLevel(level logger.LogLevel) string {
 	}
 }
 
+// normalizeGatewayLogLevel maps a user-provided log level string to a valid zerolog level.
 func normalizeGatewayLogLevel(logLevel string) string {
 	if level, ok := logger.ParseLevel(logLevel); ok {
 		return canonicalGatewayLogLevel(level)
@@ -61,7 +62,10 @@ func ResolveGatewayLogLevel(path string) string {
 
 	data, err := os.ReadFile(path)
 	if err == nil {
-		_ = json.Unmarshal(data, &cfg)
+		if jsonErr := json.Unmarshal(data, &cfg); jsonErr != nil {
+			// Malformed config — fall through to default log level.
+			_ = jsonErr
+		}
 	}
 
 	if envLevel := os.Getenv("PICOCLAW_LOG_LEVEL"); envLevel != "" {

pkg/cron/service.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"strings"
 	"sync"
 	"time"
 
@@ -68,6 +69,8 @@ type CronService struct {
 	gronx     *gronx.Gronx
 }
 
+// NewCronService creates a cron service that persists jobs to storePath.
+// onJob is called when a job fires; it may be nil if SetOnJob is used later.
 func NewCronService(storePath string, onJob JobHandler) *CronService {
 	cs := &CronService{
 		storePath: storePath,
@@ -80,6 +83,7 @@ func NewCronService(storePath string, onJob JobHandler) *CronService {
 	return cs
 }
 
+// Start begins the cron run loop in a background goroutine.
 func (cs *CronService) Start() error {
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
@@ -107,6 +111,7 @@ func (cs *CronService) Start() error {
 	return nil
 }
 
+// Stop signals the run loop to exit and waits for it to finish.
 func (cs *CronService) Stop() {
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
@@ -365,12 +370,14 @@ func (cs *CronService) getNextWakeMS() *int64 {
 	return nextWake
 }
 
+// Load reads persisted jobs from disk, replacing the in-memory store.
 func (cs *CronService) Load() error {
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
 	return cs.loadStore()
 }
 
+// SetOnJob replaces the callback invoked when a job fires.
 func (cs *CronService) SetOnJob(handler JobHandler) {
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
@@ -404,12 +411,18 @@ func (cs *CronService) saveStoreUnsafe() error {
 	return fileutil.WriteFileAtomic(cs.storePath, data, 0o600)
 }
 
+// AddJob creates and persists a new cron job. Returns an error if the
+// schedule expression is invalid or required fields are empty.
 func (cs *CronService) AddJob(
 	name string,
 	schedule CronSchedule,
 	message string,
 	channel, to string,
 ) (*CronJob, error) {
+	if strings.TrimSpace(name) == "" {
+		return nil, fmt.Errorf("job name must not be empty")
+	}
+
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
 
@@ -447,6 +460,7 @@ func (cs *CronService) AddJob(
 	return &job, nil
 }
 
+// UpdateJob replaces a job's definition and persists the change.
 func (cs *CronService) UpdateJob(job *CronJob) error {
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
@@ -464,6 +478,7 @@ func (cs *CronService) UpdateJob(job *CronJob) error {
 	return fmt.Errorf("job not found")
 }
 
+// RemoveJob deletes a job by ID and returns true if it was found.
 func (cs *CronService) RemoveJob(jobID string) bool {
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
@@ -493,6 +508,7 @@ func (cs *CronService) removeJobUnsafe(jobID string) bool {
 	return removed
 }
 
+// EnableJob toggles a job's enabled state and recomputes its next run time.
 func (cs *CronService) EnableJob(jobID string, enabled bool) *CronJob {
 	cs.mu.Lock()
 	defer cs.mu.Unlock()
@@ -522,6 +538,7 @@ func (cs *CronService) EnableJob(jobID string, enabled bool) *CronJob {
 	return nil
 }
 
+// ListJobs returns a snapshot of all jobs, optionally including disabled ones.
 func (cs *CronService) ListJobs(includeDisabled bool) []CronJob {
 	cs.mu.RLock()
 	defer cs.mu.RUnlock()
@@ -540,6 +557,7 @@ func (cs *CronService) ListJobs(includeDisabled bool) []CronJob {
 	return enabled
 }
 
+// Status returns a summary of the cron service state for diagnostics.
 func (cs *CronService) Status() map[string]any {
 	cs.mu.RLock()
 	defer cs.mu.RUnlock()

pkg/logger/logger.go

@@ -110,6 +110,7 @@ func formatFieldValue(i any) string {
 	return s
 }
 
+// SetLevel sets the global minimum log level.
 func SetLevel(level LogLevel) {
 	mu.Lock()
 	defer mu.Unlock()
@@ -117,12 +118,14 @@ func SetLevel(level LogLevel) {
 	zerolog.SetGlobalLevel(level)
 }
 
+// SetConsoleLevel adjusts the log level for console output only.
 func SetConsoleLevel(level LogLevel) {
 	mu.Lock()
 	defer mu.Unlock()
 	logger = logger.Level(level)
 }
 
+// DisableConsole silences console output by redirecting it to io.Discard.
 func DisableConsole() {
 	mu.Lock()
 	defer mu.Unlock()
@@ -130,6 +133,7 @@ func DisableConsole() {
 	logger = logger.Output(io.MultiWriter(writers...))
 }
 
+// EnableConsole restores console output after a DisableConsole call.
 func EnableConsole() {
 	mu.Lock()
 	defer mu.Unlock()
@@ -137,6 +141,7 @@ func EnableConsole() {
 	logger = logger.Output(io.MultiWriter(writers...))
 }
 
+// GetLevel returns the current global log level.
 func GetLevel() LogLevel {
 	mu.RLock()
 	defer mu.RUnlock()
@@ -186,15 +191,18 @@ func EnableFileLogging(filePath string) error {
 		return fmt.Errorf("failed to open log file: %w", err)
 	}
 
-	// Close old file if exists
+	// Close old file if exists; log but don't fail on close error
+	// since we're replacing it with a new file.
 	if logFile != nil {
-		logFile.Close()
+		if closeErr := logFile.Close(); closeErr != nil {
+			fmt.Fprintf(os.Stderr, "warning: failed to close old log file: %v\n", closeErr)
+		}
 	}
 
 	logFile = newFile
 
 	if len(writers) != 1 {
-		return fmt.Errorf("failed to configure file logging: %w", err)
+		return fmt.Errorf("invalid writers state: expected 1 writer, got %d", len(writers))
 	}
 
 	writers = append(writers, logFile)
@@ -203,12 +211,15 @@ func EnableFileLogging(filePath string) error {
 	return nil
 }
 
+// DisableFileLogging closes the log file and removes it from the output writers.
 func DisableFileLogging() {
 	mu.Lock()
 	defer mu.Unlock()
 
 	if logFile != nil {
-		logFile.Close()
+		if closeErr := logFile.Close(); closeErr != nil {
+			fmt.Fprintf(os.Stderr, "warning: failed to close log file: %v\n", closeErr)
+		}
 		logFile = nil
 	}
 	if len(writers) > 1 {
@@ -217,6 +228,8 @@ func DisableFileLogging() {
 	}
 }
 
+// ConfigureFromEnv reads PICOCLAW_LOG_FILE and PICOCLAW_LOG_LEVEL from the
+// environment and applies them to the logger configuration.
 func ConfigureFromEnv() {
 	if logFile := os.Getenv("PICOCLAW_LOG_FILE"); logFile != "" {
 		if strings.HasPrefix(logFile, "~/") {

pkg/pid/pidfile.go

@@ -61,7 +61,9 @@ func WritePidFile(homePath, host string, port int) (*PidFileData, error) {
 			logger.Warnf("not running (PID: %d) so will remove the pid file: %s", data.PID, pidPath)
 		}
 		// Stale PID file; process no longer exists → clean up.
-		os.Remove(pidPath)
+		if err := os.Remove(pidPath); err != nil && !os.IsNotExist(err) {
+			logger.Warnf("failed to remove stale pid file %s: %v", pidPath, err)
+		}
 	}
 
 	data := &PidFileData{
@@ -91,7 +93,7 @@ func WritePidFile(homePath, host string, port int) (*PidFileData, error) {
 		return nil, fmt.Errorf("failed to write pid file: %w", err)
 	}
 	if err := os.Rename(tmp, pidPath); err != nil {
-		os.Remove(tmp)
+		_ = os.Remove(tmp) // best-effort cleanup of temp file
 		return nil, fmt.Errorf("failed to rename pid file: %w", err)
 	}
 	logger.Debugf("wrote pid file: %s success", pidPath)
@@ -115,7 +117,9 @@ func ReadPidFileWithCheck(homePath string) *PidFileData {
 
 	if !isProcessRunning(data.PID) {
 		logger.Debugf("process not running, remove pid file: %s", pidPath)
-		os.Remove(pidPath)
+		if err := os.Remove(pidPath); err != nil && !os.IsNotExist(err) {
+			logger.Warnf("failed to remove stale pid file %s: %v", pidPath, err)
+		}
 		return nil
 	}
 
@@ -137,7 +141,9 @@ func RemovePidFile(homePath string) {
 	}
 
 	logger.Infof("remove pid file: %s", pidPath)
-	os.Remove(pidPath)
+	if err := os.Remove(pidPath); err != nil && !os.IsNotExist(err) {
+		logger.Warnf("failed to remove pid file %s: %v", pidPath, err)
+	}
 }
 
 // readPidFileUnlocked reads the PID file without acquiring the lock.

pkg/pid/pidfile_windows.go

@@ -23,19 +23,19 @@ func isProcessRunning(pid int) bool {
 		return false
 	}
 
-	handle, _, err := procOpenProcess.Call(
+	handle, _, _ := procOpenProcess.Call(
 		uintptr(processQueryLimitedInformation),
 		0,
 		uintptr(pid),
 	)
-	if handle == 0 || err != nil {
+	if handle == 0 {
 		return false
 	}
-	defer procCloseHandle.Call(handle)
+	defer procCloseHandle.Call(handle) //nolint:errcheck
 
 	var exitCode uint32
-	ret, _, err := procGetExitCodeProcess.Call(handle, uintptr(unsafe.Pointer(&exitCode)))
+	ret, _, _ := procGetExitCodeProcess.Call(handle, uintptr(unsafe.Pointer(&exitCode)))
-	if ret == 0 || err != nil {
+	if ret == 0 {
 		return false
 	}
 	return exitCode == stillActive

pkg/session/jsonl_backend.go

@@ -20,18 +20,21 @@ func NewJSONLBackend(store memory.Store) *JSONLBackend {
 	return &JSONLBackend{store: store}
 }
 
+// AddMessage appends a text message to the session (fire-and-forget).
 func (b *JSONLBackend) AddMessage(sessionKey, role, content string) {
 	if err := b.store.AddMessage(context.Background(), sessionKey, role, content); err != nil {
 		log.Printf("session: add message: %v", err)
 	}
 }
 
+// AddFullMessage appends a complete message (with tool calls, reasoning, etc.) to the session.
 func (b *JSONLBackend) AddFullMessage(sessionKey string, msg providers.Message) {
 	if err := b.store.AddFullMessage(context.Background(), sessionKey, msg); err != nil {
 		log.Printf("session: add full message: %v", err)
 	}
 }
 
+// GetHistory returns the message history for a session, or an empty slice on error.
 func (b *JSONLBackend) GetHistory(key string) []providers.Message {
 	msgs, err := b.store.GetHistory(context.Background(), key)
 	if err != nil {
@@ -41,6 +44,7 @@ func (b *JSONLBackend) GetHistory(key string) []providers.Message {
 	return msgs
 }
 
+// GetSummary returns the session summary, or empty string on error.
 func (b *JSONLBackend) GetSummary(key string) string {
 	summary, err := b.store.GetSummary(context.Background(), key)
 	if err != nil {
@@ -50,18 +54,21 @@ func (b *JSONLBackend) GetSummary(key string) string {
 	return summary
 }
 
+// SetSummary replaces the session summary (fire-and-forget).
 func (b *JSONLBackend) SetSummary(key, summary string) {
 	if err := b.store.SetSummary(context.Background(), key, summary); err != nil {
 		log.Printf("session: set summary: %v", err)
 	}
 }
 
+// SetHistory replaces the full message history for a session (fire-and-forget).
 func (b *JSONLBackend) SetHistory(key string, history []providers.Message) {
 	if err := b.store.SetHistory(context.Background(), key, history); err != nil {
 		log.Printf("session: set history: %v", err)
 	}
 }
 
+// TruncateHistory keeps only the last keepLast messages in a session (fire-and-forget).
 func (b *JSONLBackend) TruncateHistory(key string, keepLast int) {
 	if err := b.store.TruncateHistory(context.Background(), key, keepLast); err != nil {
 		log.Printf("session: truncate history: %v", err)

pkg/session/manager.go

@@ -2,6 +2,7 @@ package session
 
 import (
 	"encoding/json"
+	"log"
 	"os"
 	"path/filepath"
 	"strings"
@@ -32,13 +33,17 @@ func NewSessionManager(storage string) *SessionManager {
 	}
 
 	if storage != "" {
-		os.MkdirAll(storage, 0o700)
+		if err := os.MkdirAll(storage, 0o700); err != nil {
+			log.Printf("session: failed to create storage dir %s: %v", storage, err)
+		}
 		sm.loadSessions()
 	}
 
 	return sm
 }
 
+// GetOrCreate returns the session for key, creating a new empty one if it doesn't exist.
+// The returned session is never nil.
 func (sm *SessionManager) GetOrCreate(key string) *Session {
 	sm.mu.Lock()
 	defer sm.mu.Unlock()

pkg/utils/download.go

@@ -39,9 +39,13 @@ func DownloadToFile(ctx context.Context, client *http.Client, req *http.Request,
 	defer resp.Body.Close()
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		// Read a small amount for the error message.
+		// Read a small amount for the error message; ignore read errors
+		// since the HTTP status itself is the primary error signal.
 		errBody := make([]byte, 512)
-		n, _ := io.ReadFull(resp.Body, errBody)
+		n, readErr := io.ReadFull(resp.Body, errBody)
+		if readErr != nil && readErr != io.ErrUnexpectedEOF && readErr != io.EOF {
+			return "", fmt.Errorf("HTTP %d (body unreadable: %w)", resp.StatusCode, readErr)
+		}
 		return "", fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(errBody[:n]))
 	}
 
@@ -56,10 +60,12 @@ func DownloadToFile(ctx context.Context, client *http.Client, req *http.Request,
 		"path": tmpPath,
 	})
 
-	// Cleanup helper — removes the temp file on any error.
+	// Cleanup helper — best-effort removal of the temp file on error.
+	// Close/Remove errors are intentionally ignored since this only runs
+	// on failure paths where the primary error is already captured.
 	cleanup := func() {
-		_ = tmpFile.Close()
+		tmpFile.Close()
-		_ = os.Remove(tmpPath)
+		os.Remove(tmpPath)
 	}
 
 	// Optionally limit the download size.

pkg/utils/markdown.go

@@ -25,6 +25,13 @@ var skipTags = map[string]bool{
 	"nav": true, "footer": true, "aside": true, "header": true, "form": true, "dialog": true,
 }
 
+// unlikelyKeywords is allocated once at package level to avoid repeated
+// allocation on every call to isUnlikelyNode.
+var unlikelyKeywords = []string{
+	"menu", "nav", "footer", "sidebar", "cookie", "banner",
+	"sponsor", "advert", "popup", "modal", "newsletter", "share", "social",
+}
+
 func isSafeHref(href string) bool {
 	lower := strings.ToLower(strings.TrimSpace(href))
 	if strings.HasPrefix(lower, "javascript:") || strings.HasPrefix(lower, "vbscript:") ||
@@ -82,21 +89,6 @@ func isUnlikelyNode(n *html.Node) bool {
 		strings.Contains(classId, "content") {
 		return false
 	}
-	unlikelyKeywords := []string{
-		"menu",
-		"nav",
-		"footer",
-		"sidebar",
-		"cookie",
-		"banner",
-		"sponsor",
-		"advert",
-		"popup",
-		"modal",
-		"newsletter",
-		"share",
-		"social",
-	}
 	for _, keyword := range unlikelyKeywords {
 		if strings.Contains(classId, keyword) {
 			return true

pkg/utils/media.go

@@ -18,6 +18,8 @@ import (
 
 var audioExtensions = []string{".mp3", ".wav", ".ogg", ".m4a", ".flac", ".aac", ".wma"}
 
+// AudioFormat returns the audio format (e.g. "mp3", "wav") for the given file path
+// based on its extension, or an error if the format is unsupported.
 func AudioFormat(path string) (string, error) {
 	ext := strings.ToLower(filepath.Ext(path))
 	for _, supportedExt := range audioExtensions {