Address file growth concern from #711 review: logical truncation via
skip offset is fast but leaves dead lines on disk indefinitely.
Compact() rewrites the JSONL file keeping only active messages, using
the same temp+rename pattern for crash safety. No-op when skip == 0.
The caller (lifecycle manager or agent loop) decides when to trigger
compaction — e.g. when skipped lines exceed active lines.
Read existing sessions/*.json files, convert to JSONL format, and
rename originals to .json.migrated as backup. The migration is
idempotent — second runs skip already-migrated files.
Session keys are read from JSON content (not filenames) so that
sanitized names like telegram_123 correctly map back to telegram:123.
Cover all Store interface methods plus edge cases:
- Basic roundtrip, ordering, empty session, tool calls
- Logical truncation (keep last N, keep zero, keep more than exist)
- SetHistory replacing all + resetting skip offset
- Crash recovery with partial JSON lines
- Persistence across store instances
- Concurrent add+read (10 goroutines x 20 msgs)
- Simulated #704 race (summarizer vs main loop)
- Benchmarks for AddMessage and GetHistory (100/1000 msgs)
Add JSONLStore that persists sessions as .jsonl files (one message per
line) plus .meta.json for summary and truncation offset.
Key design decisions:
- Append-only writes — no full-file rewrites on AddMessage
- Logical truncation via skip offset instead of physical deletion
- Per-session mutex for safe concurrent access
- Crash recovery: malformed trailing lines are silently skipped
- Atomic metadata writes using temp+rename
Zero new dependencies — pure stdlib.
Refs #711
Introduce a backend-agnostic Store interface in pkg/memory/ that maps
one-to-one with the current SessionManager API. Each method is atomic
— no separate Save() call needed.
Refs #711
* 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>
* refactor(cli): migrate to Cobra-based command structure
Refactor CLI to use Cobra instead of manual os.Args parsing.
- Introduce root command and structured subcommands under cmd/picoclaw/internal
- Convert agent, auth, cron, gateway, migrate, onboard, skills, status and version to Cobra commands
- Replace manual flag parsing with Cobra flags
- Remove direct os.Args usage from command handlers
- Keep existing command behavior and output semantics
This change focuses on CLI structure and maintainability.
No business logic changes intended.
* chore(cli): remove version2 alias and make cobra a direct dependency
* test(cli): add basic command tests
- Add tests for CLI command tree and flag parsing
- Align LDFLAGS injection path for version info
- Remove unused manual help function
* test: migrate command tests to testify assertions
Replace standard library testing error checks (t.Error*, t.Fatalf)
with assert/require from stretchr/testify across all cobra command tests
for improved readability and consistency.
* fix(cli): make linter happy
* test: avoid duplication in windows config path test
* test: simplify allowed command checks using slices.Contains
* fix(skills): register subcommands during command construction
- Move subcommand registration out of PersistentPreRunE
- Ensure `picoclaw skills <subcommand>` resolves correctly
- Minor install command and test cleanups
* refactor(cli): address review feedback and improve command clarity
* fix(authLogoutCmd): rm os.Exit
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).
