- WhatsApp Start(): use deferred cleanup to nil out c.client/c.container
and disconnect/close resources on any error after struct fields are
assigned, preventing stale references and double-close in Stop()
- handleReasoning: treat bus.ErrBusClosed as an expected condition
(DEBUG level) alongside context timeout/cancel, avoiding WARN noise
during normal shutdown
- WhatsApp Send(): detect unpaired state (Store.ID == nil) and return
ErrTemporary instead of attempting to send while QR login is pending
- handleReasoning: check the returned error type (DeadlineExceeded /
Canceled) instead of ctx.Err() to decide log level, so pubCtx
timeouts on a full bus are correctly classified as expected
- Test: fill bus with a short-timeout loop instead of hardcoding the
buffer size (64), making the test resilient to buffer size changes
Move the stopping check and wg.Add(1) inside reconnectMu in
eventHandler, and set the stopping flag under the same lock in Stop().
This makes the two operations atomic with respect to each other,
preventing the race where:
1. eventHandler checks stopping (false)
2. Stop() sets stopping=true and enters wg.Wait() (wg is 0)
3. eventHandler calls wg.Add(1) → panic or goroutine leak
- Use c.runCtx for GetQRChannel so the QR producer is canceled on Stop()
- Add atomic stopping guard to prevent wg.Add/wg.Wait race in eventHandler
- Make Stop() context-aware: disconnect client before waiting, respect ctx deadline
- Reduce reasoning publish log noise: use debug level for expected ctx errors
- Add test for handleReasoning when outbound bus is full (timeout path)
Add a 5-second timeout to handleReasoning's PublishOutbound call so
fire-and-forget goroutines do not block indefinitely when the outbound
bus channel is full. Reasoning output is best-effort; on timeout the
publish is abandoned with a warning log instead of holding the
goroutine alive.
Fixes goroutine leak introduced in #802.
- Move runCtx/runCancel creation before event handler registration and
QR loop so Stop() can cancel at any point during startup
- Replace blocking QR event loop in Start() with a background goroutine
that selects on runCtx.Done(), preventing Start() from hanging
indefinitely when waiting for QR scan
- Track all background goroutines (QR handler, reconnect) with
sync.WaitGroup; Stop() waits for them to finish before releasing
client/container resources
- Cancel runCtx on error paths in Start() to avoid leaked contexts
Fixes resource leak introduced in #655.
Define PlaceholderCapable, TypingCapable, and ReactionCapable interfaces
and have BaseChannel.HandleMessage auto-detect and trigger all three as
independent pipelines on inbound messages. This replaces the scattered
manual orchestration code in each channel's handleMessage with a single
unified dispatch in the framework layer.
Changes:
- Add PlaceholderCapable interface to interfaces.go
- Add ReactionCapable + RecordReactionUndo to interfaces.go
- BaseChannel.HandleMessage auto-triggers Typing → Reaction → Placeholder
- Manager gains reactionUndos sync.Map with TTL janitor cleanup
- Telegram: extract SendPlaceholder from manual code, add StartTyping
- Discord: add SendPlaceholder + StartTyping
- Pico: add SendPlaceholder (uses Pico Protocol message.create)
- Slack: extract ReactToMessage from manual code
- OneBot: extract ReactToMessage, remove leaked pendingEmojiMsg sync.Map
- LINE: move group-chat guard into StartTyping, remove manual orchestration
- Config: add Placeholder to PicoConfig; remove from Slack/LINE/OneBot
(no MessageEditor, so placeholder config was dead code)
Port changes that were applied to the old pkg/channels/*.go files on main
to their new locations in channel subpackages:
- telegram: precompile regex, var transcribedText, GetModelName()
- discord: var transcribedText declaration
- onebot: resp.Body.Close(), "canceled" spelling, remove empty line
- slack: named return values in parseSlackChatID
- wecom: remove sendMarkdownMessage dead code
- whatsapp: resp.Body.Close() after Dial
- gateway/helpers: remove unused errors import
Prevents potential backpressure under load when multiple channels
publish concurrently in gateway mode, where SDK callbacks blocking
on a full buffer can cause message loss or timeouts.
- Replace stdlib log.Printf with logger.InfoCF/WarnCF for consistency
with the rest of the codebase (addresses @nikolasdehor review point #3)
- ReleaseAll: clean refToScope/refs mappings even if refs entry is missing
- CleanExpired: guard refToScope lookup before scope cleanup
- Add TestReleaseAllCleansMappingsIfRefsMissing for robustness
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address review feedback from @alexhoshina and Codex:
- Log sendLoading errors in ticker goroutine instead of discarding
- Only start typing indicator when PlaceholderRecorder is available
to avoid wasted API calls and unnecessary goroutine creation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat(skills): add retry mechanism for HTTP requests
Implement a retry mechanism with exponential backoff for HTTP requests in the skill installer. This improves reliability when fetching skills from GitHub by automatically retrying failed requests up to 3 times.
Add comprehensive tests to verify retry behavior under different scenarios including success on different attempts and proper delay between retries.
* fix: improve http request retry logic with status code checks
Add shouldRetry helper function to determine retryable status codes.
Close response body between retry attempts and break early for non-retryable status codes.
* refactor: remove unused BuiltinSkill struct
The struct was not being used anywhere in the codebase, so it's safe to remove it to reduce clutter and improve maintainability.
* refactor(http): move retry logic to utils package
Extract HTTP retry functionality from skills package to utils for better reusability
Add context-aware sleep function and comprehensive tests
* refactor(http): extract retry delay unit to variable
Extract hardcoded retry delay unit to a variable for better testability and flexibility. Update tests to use milliseconds for faster execution while maintaining the same behavior.
* test(http_retry): remove t.Parallel from test cases
* test(http_retry): remove redundant test cases for retry success
The removed test cases for success on second and third attempts were redundant since the retry logic is already covered by other tests. This simplifies the test suite while maintaining coverage.
Change the default value of session.dm_scope from "main" to
"per-channel-peer" to provide better conversation isolation by
default. This prevents context leakage between different users
and channels.
Implement TypingCapable interface for LINE channel using the
loading animation API (1:1 chats only, no group support).
- Add StartTyping() with 50s periodic refresh and context-based stop
- Integrate PlaceholderRecorder.RecordTypingStop in processEvent
- Skip RecordPlaceholder (LINE has no message edit API)
- Change sendLoading to accept context and return error
- Relax callAPI status check from 200 to 2xx range
Design consulted with Codex (GPT-5.2).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address Codex (GPT-5.2) review feedback:
- Start: guard against Interval<=0 or MaxAge<=0 to prevent
time.NewTicker panic on misconfiguration
- ReleaseAll: split into two phases (collect under lock, delete
after unlock) matching CleanExpired pattern
- ReleaseAll: log file removal errors
- Add TestStartZeroIntervalNoPanic and TestStartZeroMaxAgeNoPanic
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- CleanExpired: split into two phases — collect expired entries under
lock, then delete files after releasing the lock to minimize contention
- CleanExpired: guard against zero MaxAge (no-op if unconfigured)
- CleanExpired: log file removal errors instead of silently ignoring
- Start: protect with startOnce to prevent multiple goroutines
- Stop: rename once -> stopOnce for clarity
- cmd_gateway: call mediaStore.Stop() on error path after Start()
- Add TestCleanExpiredZeroMaxAge and double-Start test
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: remove redundant tools definitions from system prompt
Tools are already provided to the LLM via JSON schema through
ToProviderDefs(), so the text-based tools section in the system
prompt is redundant.
This removes the buildToolsSection() logic and the tools field
from ContextBuilder, reducing system prompt length while maintaining
the "ALWAYS use tools" rule reminder.
Fixes#731
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* fix: correct spelling 'initialized' (was 'initialised')
---------
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Whitespace is a linter that checks for unnecessary newlines at the start and end of functions, if, for, etc.
Signed-off-by: Kai Xia <kaix+github@fastmail.com>
Avoid rebuilding the entire system prompt on every BuildMessages() call
by caching the static portion (identity, bootstrap, skills summary,
memory) and only recomputing it when workspace source files change.
Key changes:
- ContextBuilder caches the static prompt behind an RWMutex with
double-checked locking. Source file changes are detected via cheap
os.Stat mtime checks so no explicit invalidation is needed.
- Track file existence at cache time (existedAtCache map) so that
newly created or deleted bootstrap/memory files also trigger a
rebuild — the old modifiedSince() silently returned false on
os.IsNotExist.
- Walk the skills directory recursively with filepath.WalkDir to
catch content-only edits at any nesting depth; directory mtime
alone misses in-place file modifications on most filesystems.
- ToolRegistry.sortedToolNames() sorts tool names before iteration,
ensuring deterministic tool definition order across calls — a
prerequisite for LLM-side prefix/KV cache reuse.
- Merge all context (static + dynamic + summary) into a single
system message for provider compatibility: the Anthropic adapter
extracts messages[0] as the top-level system parameter, and Codex
reads only the first system message as instructions.
- Fix a data race in BuildMessages() where cachedSystemPrompt was
read without holding the lock in a debug log statement.
- Add tests: single system message invariant, mtime auto-invalidation,
new-file creation detection, skill file content change, explicit
InvalidateCache, cache stability, concurrent access (20 goroutines
x 50 iterations, passes go test -race), and a benchmark.
Rename fastID() to uniqueID() with a security caveat comment clarifying
the ID is not cryptographically secure, and add unit tests for the
refactored index-based split helper functions.
The spawn tool accepts empty strings as valid task arguments, which
causes a subagent to run with no meaningful work. The subagent's
completion message is then routed back to the originating channel
(e.g. Signal, Discord), where the main agent processes it and may
hallucinate an unrelated response that gets sent to users.
Validate that the task parameter is non-empty after trimming whitespace.
Related: #545
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
HTTP timeouts (context deadline exceeded, Client.Timeout) were
incorrectly classified as context window errors, triggering useless
history compression. Replace broad substring checks ("context",
"token", "length") with specific patterns for real context limit
errors and explicitly exclude timeout errors from that path.
Additionally, timeout errors were not retried at all — the retry
loop only handled context window errors. Now timeouts are retried
up to 2 times with exponential backoff (5s, 10s).
* cleanup dead code.
Signed-off-by: Kai Xia <kaix+github@fastmail.com>
* add these two back with flag.
Signed-off-by: Kai Xia <kaix+github@fastmail.com>
* fix ci
Signed-off-by: Kai Xia <kaix+github@fastmail.com>
* remove this confusing line
Signed-off-by: Kai Xia <kaix+github@fastmail.com>
* make fmt
Signed-off-by: Kai Xia <kaix+github@fastmail.com>
* remove unused method.
picked up by golangci-lint run
Signed-off-by: Kai Xia <kaix+github@fastmail.com>
---------
Signed-off-by: Kai Xia <kaix+github@fastmail.com>
Walk backwards over preceding tool messages to find the nearest assistant
with ToolCalls, instead of only checking the immediate predecessor. Add
unit tests for sanitizeHistoryForProvider covering key edge cases.
Add background TTL-based cleanup (L2 safety net) directly into
FileMediaStore so file deletion and in-memory ref removal happen
atomically under the same mutex, preventing dangling references.
- Add storedAt timestamp and refToScope reverse map to mediaEntry
- Add CleanExpired() for atomic TTL-based expiration
- Add Start()/Stop() for background goroutine lifecycle
- Add MediaCleanupConfig (enabled, max_age, interval) to config
- Wire up in cmd_gateway.go with config-driven defaults
- Add 8 new tests including concurrent cleanup safety
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
