Skip to content

feat: adaptive learning system — skill synthesis, user profiles, session search#1187

Open
smkrv wants to merge 9 commits intonearai:mainfrom
smkrv:feat/adaptive-learning
Open

feat: adaptive learning system — skill synthesis, user profiles, session search#1187
smkrv wants to merge 9 commits intonearai:mainfrom
smkrv:feat/adaptive-learning

Conversation

@smkrv
Copy link
Contributor

@smkrv smkrv commented Mar 14, 2026

Hey folks, don't think I've lost my mind here, but I'd like to propose this level-up PR with an adaptive learning mechanism that will make the whole project a bit smarter and the routine a bit easier.

I've tried to test this as thoroughly as I could - ran it through 30+ full review-and-fix iterations across Opus and Codex (not code generation - testing, reviewing, and fixing), with multi-agent review panels (Architecture, Code Style, Security, Product Owner, Lead Dev, UI/UX, CPO, and others - all Blocking/High/Critical findings resolved), and also tested locally on my own agents in my own environment.

But I could have missed things (I definitely have :D), including edge cases that only show up under real-world conditions at scale.

Fresh eyes are very welcome.

Would really appreciate help with testing and reviewing all of this - no matter how good LLM-powered code generation and review gets, pulling something like this off solo is tough :) Would be awesome to see this feature get some life and evolve further as a real level-up for the project. And yes,

I swear I'm not crazy — I just went way too deep down this rabbit hole and at some point there was no turning back, so here we are.


What this does

This PR adds an Adaptive Learning System — three independently testable subsystems that let IronClaw autonomously learn from experience while architecturally guaranteeing that learned knowledge cannot leak secrets.

The Three Pillars

1. Autonomous Skill Synthesis (src/learning/)

The agent watches its own successful interactions. When it completes a complex multi-tool task (3+ tool calls with quality score ≥75, diverse tool combinations with ≥2 unique tools, or high quality completions scoring 90+), a background worker synthesizes a reusable SKILL.md from the interaction — via LLM, not templates. The generated skill passes through 6 layers of security validation before the user even sees it. The user must explicitly approve before it goes live. No auto-deployment, ever.

2. Encrypted User Profile Engine (src/user_profile/)

An evolving user model built from conversations. The ProfileDistiller extracts durable facts (timezone, expertise, communication style) via LLM, encrypts them with AES-256-GCM (per-fact HKDF-derived keys, random 32-byte salt, master key from OS keychain or SECRETS_MASTER_KEY env var — same crypto as credential storage), and injects them into the system prompt on next turn. The agent remembers you across sessions without storing anything in plaintext.

3. Session Search (src/db/libsql/session_search.rs, src/db/postgres.rs)

FTS5 (libSQL) / tsvector (PostgreSQL) full-text search over conversation summaries with vector embedding support for future hybrid search. The session_search tool lets the agent recall prior work, decisions, and context from previous conversations.

Architecture Diagrams

Full Architecture Pipeline

Full Architecture Pipeline

Skill Lifecycle State Machine

Skill Lifecycle State Machine

Runtime Sequence Flow

Runtime Sequence Flow

Security Architecture

Security Architecture

Database Schema (V13)

Database Schema

Security Architecture (6 defense-in-depth layers)

All learned data flows through IronClaw's existing security primitives:

  1. Content Wrappingwrap_external_content() on every untrusted input before it reaches an LLM prompt
  2. Threat Scanningscan_content_for_threats() with NFKC unicode normalization, prompt injection patterns, role manipulation detection
  3. Structural ValidationSkillValidator enforces YAML frontmatter, 16 KiB size limit, 256-char description limit, name/content/description threat scanning
  4. Encryption at Rest — Profile facts encrypted with AES-256-GCM, per-fact salt, master key never leaves OS keychain or env var
  5. Trust BoundariesSkillSource::Synthesized forces Installed trust (read-only tools, treated as suggestions), path traversal prevention
  6. User Approval GateApprovalRequirement::Always on skill_approve for accept action — the LLM cannot self-approve its own generated skills (reject is safe and doesn't require approval)

Architecture Decisions

  • Standalone store traits (SessionSearchStore, UserProfileStore, LearningStore) — NOT added to Database supertrait. Injected via Arc<dyn T>. This keeps the existing DB implementations untouched and lets the system start without these features.
  • Background worker via bounded mpsc(32) — event-driven consumer pattern. Backpressure built in. Worker dies when sender is dropped.
  • Heuristic quality scoring — no LLM call needed. Simple formula: base score (50 no errors, 20 with errors) + tool diversity bonus + turn bonus + volume bonus, capped at 100.
  • Dual-backend — PostgreSQL (V13 migration) + libSQL (incremental migration V13) as per project requirements.

Files Changed

  • 6 new modules: src/learning/ (7 files), src/user_profile/ (5 files)
  • 6 new tools: session_search, skill_list_pending, skill_approve, profile_view, profile_edit, profile_clear
  • 3 new store traits in src/db/mod.rs with implementations for both PostgreSQL and libSQL
  • V13 migration for both backends
  • Integration in dispatcher.rs: profile injection via append_system_context() after system prompt is set, learning event emission after turn completion
  • SkillSource::Synthesized variant in src/skills/registry.rs with forced Installed trust

smkrv added 7 commits March 14, 2026 23:34
Add V13 PostgreSQL and libSQL migrations with three new tables:
- session_summaries (FTS + vector search for session history)
- user_profile_facts (AES-256-GCM encrypted user profile storage)
- synthesized_skills (audit log with approval workflow)

Add standalone store traits (SessionSearchStore, UserProfileStore,
LearningStore) injected via Arc<dyn T> through DatabaseHandles —
NOT added to Database supertrait to preserve backward compatibility.

Implement both PostgreSQL and libSQL backends with full dialect parity
including CHECK constraints, FTS5 triggers, composite indexes.

Security: IDOR protection (user_id filtering on all get/update methods),
FTS5 query sanitization, proper error propagation (no unwrap_or).

Add LearningConfig and UserProfileConfig with resolve() pattern,
integrated into Config::build() and Config::for_testing().
New src/learning/ module with:
- PatternDetector: identifies synthesis candidates (complex tool chains,
  novel combinations, user-requested, high quality completions)
- SkillValidator: structural + safety validation of generated SKILL.md
  (parser, threat scanning, size limits, description/name checks)
- SkillSynthesizer: trait + LLM-powered implementation with system/user
  message separation and wrap_external_content on all user-origin data
- LearningEvent: typed event for background worker integration

Security additions to ironclaw_safety crate:
- scan_content_for_threats(): regex-based threat pattern scanner with
  real NFKC unicode normalization (unicode-normalization crate) and
  zero-width character stripping
- Patterns: prompt injection, credential exfiltration, data theft,
  deception, SSH backdoor, destructive commands, secret references,
  role manipulation, prompt delimiter injection


19 learning tests + 90 safety tests pass. Clippy 0 warnings.
New src/user_profile/ module with:
- UserProfile, ProfileFact, FactCategory, FactSource types with
  prompt formatting (BTreeMap sorted categories, char-boundary safe
  truncation, capitalized headings)
- EncryptedProfileEngine: trait + AES-256-GCM implementation via
  SecretsCrypto (HKDF per-fact salt, safety scan on key+value)
- ProfileDistiller: LLM-powered fact extraction from conversations
  with system/user message separation, wrap_external_content on
  both user messages AND existing profile facts, key format validation
  (alphanumeric+underscore, max 64), value length limit (512 chars),
  per-run fact cap (5), byte limit on input (4KB), threat scanning

Security: existing profile facts wrapped before LLM injection,
first message always included even if exceeding byte limit,
fact key/value validation prevents injection through stored data.

11 tests (types roundtrip, truncation, parsing, safety rejection).
Clippy 0 warnings.
New tools for the adaptive learning system:
- session_search: FTS search over past conversation summaries
- skill_list_pending: list synthesized skills awaiting approval
- skill_approve: approve/reject synthesized skills with file write

Security hardening:
- Path traversal prevention: is_safe_skill_name() validation +
  starts_with() defense-in-depth check on resolved path
- PROTECTED_TOOL_NAMES: all 3 tools registered to prevent shadowing
- ApprovalRequirement::Always only for "accept", "reject" is safe
- tokio::fs for non-blocking file I/O (not std::fs)
- Errors propagated to user (no silent failure on disk write)

Added register_learning_tools() to ToolRegistry.
SkillSource::Synthesized variant for auto-synthesized skills:
- discover_all() scans installed_skills/auto/ with Installed trust
- validate_remove() allows removal of synthesized skills
- CLI format_source() displays "synthesized" label

Profile management tools (profile_view, profile_edit, profile_clear):
- View all learned facts with category/key/value/confidence
- Manually add facts with Explicit source and max confidence
- Remove specific facts by category/key

Dispatcher integration:
- AgentDeps extended with learning_tx, profile_engine, user_profile_config
- Profile injection into system prompt via Reasoning::append_system_context()
- Double safety: sanitize_tool_output + scan_content_for_threats on composed text
- All AgentDeps construction sites updated (main, testing, dispatcher tests, test rigs)

PROTECTED_TOOL_NAMES updated with all 6 new tools.
Clippy 0 warnings. 3042 tests pass.
Background worker (spawn_learning_worker):
- Bounded mpsc channel (capacity 32) for LearningEvent reception
- PatternDetector evaluation → LLM synthesis → SkillValidator check
- Skills failing validation are discarded (not written to DB)
- Graceful shutdown on sender drop

Heuristic quality_score (no LLM call required):
- FNV-1a 64-bit hash for content deduplication
- Base 50/20 + diversity/turn/volume bonuses, capped at 100

Full wiring in main.rs and app.rs:
- AppComponents extended with learning store handles
- learning_tx spawned when LEARNING_ENABLED=true
- profile_engine created when USER_PROFILE_ENABLED=true + master key

Security hardening:
- skill_approve: blocks skills with safety_scan_passed=false
- skill_approve: re-validates content before writing to disk
- format_for_prompt: strips newlines from fact values
- profile_edit: redacts value from ToolOutput (SSE/log safe)
- FTS query length limit (512 chars) prevents DoS

4 unit tests for quality scoring. Clippy 0 warnings.
…tcher events

SkillStatus enum (replaces string literals):
- Typed enum with Pending/Accepted/Rejected variants
- as_str()/from_str_opt() for DB serialization
- All trait signatures, implementations, tools, and worker updated

libSQL transaction safety:
- upsert_session_summary: INSERT + SELECT-back wrapped in transaction
- upsert_profile_fact: INSERT + SELECT-back wrapped in transaction

Dispatcher integration:
- LearningEvent sent after successful turn via try_send (non-blocking)
- Tool names extracted from reasoning context tool_calls
- ProfileDistiller auto-called in background tokio::spawn task
- Profile facts extracted and stored after each turn

PostgreSQL FTS query validation (parity with libSQL):
- Empty query rejection and 512-char length limit

Clippy 0 warnings. 3044 tests pass.
@github-actions github-actions bot added scope: agent Agent core (agent loop, router, scheduler) scope: channel/cli TUI / CLI channel scope: tool Tool infrastructure scope: tool/builtin Built-in tools scope: db Database trait / abstraction scope: db/postgres PostgreSQL backend scope: db/libsql libSQL / Turso backend scope: llm LLM integration scope: dependencies Dependency updates size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: regular 2-5 merged PRs labels Mar 14, 2026
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant 'level-up' to the project by integrating an adaptive learning system. This system empowers the agent to autonomously learn from its interactions, personalize user experiences through encrypted profiles, and efficiently retrieve past conversational context. The changes are designed to make the agent smarter and more adaptable, while rigorously maintaining security and privacy standards through multiple layers of validation and encryption.

Highlights

  • Adaptive Learning System Introduced: Implemented a comprehensive adaptive learning system featuring autonomous skill synthesis, an encrypted user profile engine, and session search capabilities to enhance agent intelligence and efficiency.
  • Autonomous Skill Synthesis: Enabled the agent to automatically generate reusable SKILL.md files from successful, complex multi-tool interactions, subject to multi-layered security validation and explicit user approval.
  • Encrypted User Profile Engine: Developed a system to distill and store durable user facts (e.g., timezone, expertise) from conversations, encrypting them with AES-256-GCM and injecting them into system prompts for personalized interactions without plaintext storage.
  • Session Search Functionality: Added full-text search (FTS5/tsvector) over conversation summaries, with vector embedding support for future hybrid search, allowing the agent to recall prior work and context.
  • Enhanced Security Architecture: Integrated six defense-in-depth layers for learned data, including content wrapping, threat scanning, structural validation, encryption at rest, trust boundaries, and user approval gates for synthesized skills.
  • New Database Migrations and Store Traits: Introduced V13 database migrations for both PostgreSQL and libSQL, adding tables for session summaries, user profile facts, and synthesized skills, along with new standalone store traits.
  • New Built-in Tools: Added six new built-in tools: session_search, skill_list_pending, skill_approve, profile_view, profile_edit, and profile_clear to manage the new learning and user profile features.
Changelog
  • .gitignore
    • Added entries for .DS_Store and local architecture/plan documentation directories.
  • Cargo.lock
    • Updated dependencies to include 'unicode-normalization'.
  • crates/ironclaw_safety/Cargo.toml
    • Added 'unicode-normalization' dependency for enhanced security scanning.
  • crates/ironclaw_safety/src/lib.rs
    • Implemented scan_content_for_threats function to detect prompt injection, credential exfiltration, and other malicious patterns.
    • Added normalize_for_scanning function to handle Unicode normalization and zero-width character stripping for robust threat detection.
    • Extended unit tests for new threat scanning functionality.
  • migrations/V13__learning_system.sql
    • Created new database tables: session_summaries for conversation search, user_profile_facts for encrypted user data, and synthesized_skills for skill audit logs.
  • src/agent/agent_loop.rs
    • Extended AgentDeps struct to include optional learning_tx (for background worker), profile_engine, and user_profile_config.
  • src/agent/dispatcher.rs
    • Modified agent dispatch logic to inject user profile data into the system prompt if enabled and available.
    • Added logic to emit LearningEvents after successful turn completion, triggering skill synthesis and profile distillation.
    • Implemented auto-distillation of user profile facts at regular intervals.
  • src/app.rs
    • Updated AppComponents to hold new learning system store handles (session search, user profile, learning store).
    • Modified AppBuilder to capture these store handles during database connection before type erasure.
  • src/cli/skills.rs
    • Added formatting for the new Synthesized skill source type.
  • src/config/learning.rs
    • Added new configuration struct LearningConfig to manage adaptive learning parameters like minimum tool calls, unique tools, quality score, and skill limits.
  • src/config/mod.rs
    • Integrated learning and user_profile modules into the main configuration system.
    • Added LearningConfig and UserProfileConfig to the main Config struct.
  • src/config/user_profile.rs
    • Added new configuration struct UserProfileConfig to manage user profile engine parameters like enabled status, max prompt characters, distillation interval, and max facts.
  • src/db/libsql/learning.rs
    • Implemented the LearningStore trait for the LibSQL backend, providing persistence for synthesized skills.
  • src/db/libsql/mod.rs
    • Included new LibSQL modules for learning, session search, and user profiles.
  • src/db/libsql/session_search.rs
    • Implemented the SessionSearchStore trait for the LibSQL backend, including FTS5 search and a vector search fallback.
  • src/db/libsql/user_profile.rs
    • Implemented the UserProfileStore trait for the LibSQL backend, handling encrypted user profile facts.
  • src/db/libsql_migrations.rs
    • Added V13 migration for LibSQL, creating session_summaries, user_profile_facts, and synthesized_skills tables with FTS5 virtual table and triggers.
  • src/db/mod.rs
    • Extended DatabaseHandles to include session_search_store, user_profile_store, and learning_store as Arc<dyn T> trait objects.
    • Modified connect_with_handles to create and store these standalone learning system trait objects from concrete backend types.
  • src/db/postgres.rs
    • Implemented SessionSearchStore, UserProfileStore, and LearningStore traits for the PostgreSQL backend, including pgvector support for embeddings.
  • src/learning/candidate.rs
    • Defined SynthesisCandidate struct to represent interactions suitable for skill synthesis.
    • Defined DetectionReason enum to categorize why an interaction was flagged for synthesis.
  • src/learning/detector.rs
    • Implemented PatternDetector to evaluate completed interactions and identify those worthy of skill synthesis based on configured thresholds.
  • src/learning/error.rs
    • Defined LearningError enum for error handling within the adaptive learning system.
  • src/learning/mod.rs
    • Created the learning module, encapsulating candidate detection, synthesis, validation, and worker logic.
  • src/learning/synthesizer.rs
    • Defined SkillSynthesizer trait and LlmSkillSynthesizer implementation for LLM-powered skill generation.
    • Implemented prompt construction for skill synthesis, including safety wrapping for user-provided context.
  • src/learning/validator.rs
    • Implemented SkillValidator to enforce structural correctness, size limits, and safety checks on synthesized skills before persistence.
  • src/learning/worker.rs
    • Implemented spawn_learning_worker to run as a background task, processing LearningEvents, detecting synthesis candidates, and managing skill lifecycle.
    • Added heuristic_quality_score function to evaluate interaction quality.
  • src/lib.rs
    • Added learning and user_profile modules to the crate root.
  • src/llm/reasoning.rs
    • Added append_system_context method to Reasoning for injecting additional context (like user profiles) into the system prompt.
  • src/main.rs
    • Integrated the adaptive learning system by initializing the learning worker and user profile engine based on configuration.
    • Registered new learning and user profile tools with the tool registry.
  • src/skills/mod.rs
    • Added SkillSource::Synthesized variant to denote skills generated by the learning system.
  • src/skills/registry.rs
    • Modified skill discovery logic to include auto-synthesized skills from a dedicated directory, with Installed trust.
  • src/testing/mod.rs
    • Updated test harness to include default learning_tx, profile_engine, and user_profile_config.
  • src/tools/builtin/learning_tools.rs
    • Implemented SkillListPendingTool to list skills awaiting user approval.
    • Implemented SkillApproveTool to allow users to accept or reject synthesized skills, including safety checks and filesystem persistence for accepted skills.
  • src/tools/builtin/mod.rs
    • Exported new learning and profile tools for use in the tool registry.
  • src/tools/builtin/profile_tools.rs
    • Implemented ProfileViewTool to display user profile facts.
    • Implemented ProfileEditTool to allow manual addition or update of profile facts.
    • Implemented ProfileClearTool to remove specific or all facts from a category.
  • src/tools/builtin/session_search.rs
    • Implemented SessionSearchTool for searching past conversation summaries.
  • src/tools/registry.rs
    • Added new learning and user profile tools to the tool registry.
    • Included new tool names in PROTECTED_TOOL_NAMES to prevent unauthorized modification.
  • src/user_profile/distiller.rs
    • Implemented ProfileDistiller to extract structured profile facts from user messages using an LLM.
    • Incorporated safety wrapping and fact validation to prevent injection and ensure data integrity.
  • src/user_profile/engine.rs
    • Defined UserProfileEngine trait and EncryptedProfileEngine implementation for managing encrypted user profiles.
    • Handles encryption/decryption of profile facts using SecretsCrypto and enforces fact limits and safety checks.
  • src/user_profile/error.rs
    • Defined UserProfileError enum for error handling within the user profile system.
  • src/user_profile/mod.rs
    • Created the user_profile module, containing distiller, engine, error, and types for user profiles.
  • src/user_profile/types.rs
    • Defined FactCategory, FactSource, ProfileFact, and UserProfile structs for structured user data.
    • Implemented format_for_prompt for UserProfile to generate a concise, truncated representation for LLM injection.
  • tests/support/gateway_workflow_harness.rs
    • Updated GatewayWorkflowHarness to include default learning and user profile components for testing.
  • tests/support/test_rig.rs
    • Updated TestRigBuilder to include default learning and user profile components for testing.
Activity
  • The author, smkrv, has invested significant effort, running over 30 full review-and-fix iterations with multiple AI agents (Opus, Codex) acting as reviewers (Architecture, Code Style, Security, Product Owner, Lead Dev, UI/UX, CPO).
  • All Blocking/High/Critical findings identified by these multi-agent review panels have been resolved.
  • Local testing was also performed on the author's own agents and environment.
  • smkrv acknowledges that despite thorough testing, edge cases might exist and welcomes fresh eyes and testing from other contributors.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This is an impressive and ambitious pull request that introduces a sophisticated adaptive learning system. The architecture is well-designed with clear separation of concerns between skill synthesis, user profiles, and session search. The focus on security is particularly noteworthy, with multiple layers of defense including content wrapping, threat scanning, structural validation, and strong encryption. The code is high quality, well-documented, and includes good test coverage. I've identified a couple of potential race conditions that could lead to data corruption or bypassing resource limits, but overall this is an excellent contribution.

smkrv added 2 commits March 15, 2026 02:48
Gemini Code Assist correctly identified that the 8-character hash prefix
used for auto-skill directory names could collide. Using the skill's
UUID (guaranteed unique) as the directory name eliminates this risk.
The skill's activation name is read from SKILL.md frontmatter, not
the directory name.

Removed is_safe_skill_name() — no longer needed since UUID paths
are inherently safe (no traversal possible).
@smkrv
Copy link
Contributor Author

smkrv commented Mar 14, 2026

Re: TOCTOU in max_facts_per_user check (engine.rs:145)

Good catch on the race condition — this is a real concern at scale. Here's the current mitigation and why a per-user mutex isn't the right fit yet:

Current state:

  • libSQL (default backend): WAL mode serializes all writes through a single writer with 5s busy timeout. Concurrent store_fact calls for the same user are physically serialized at the DB level — the race cannot occur.
  • PostgreSQL: The distiller runs in a single tokio::spawn per turn per user in the dispatcher. Two concurrent distillations for the same user_id would require two simultaneous turns completing for the same user, which the session model (single active thread per session) makes unlikely.

Why not per-user mutex:

  • DashMap<String, Mutex<()>> in the engine creates runtime state that doesn't survive restarts, grows unboundedly (no eviction), and adds complexity for a race that the DB layer already mitigates.
  • The proper fix is an atomic DB-level check: INSERT ... WHERE (SELECT COUNT(*) ...) < limit — but that requires a new trait method + dual-backend implementation, which is better as a follow-up.

What we do have:

  • is_update check allows UPDATE at the limit (won't block editing existing facts)
  • Fact key uniqueness (UNIQUE(user_id, agent_id, category, fact_key)) prevents true duplicates
  • MAX_FACTS_PER_RUN = 5 caps each distillation batch

Happy to add the atomic DB-level constraint as a follow-up if the team thinks the current guardrails aren't sufficient.

Copy link
Collaborator

@zmanian zmanian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: REQUEST CHANGES

Strong implementation quality and well-thought-out security architecture across ~4,500 lines. Three independent subsystems (skill synthesis, user profiles, session search), all disabled by default. But functional gaps need addressing.

What's Good

  • Architecture: Standalone store traits (SessionSearchStore, UserProfileStore, LearningStore) rather than extending the Database supertrait -- excellent decision, avoids breaking existing implementations.
  • Dual-backend: Both PostgreSQL + libSQL correctly implemented with proper dialect handling (FTS5 with sync triggers vs tsvector GENERATED ALWAYS).
  • Security defense-in-depth: wrap_external_content() on user data, scan_content_for_threats() with NFKC normalization + zero-width stripping, per-fact HKDF-derived AES-256-GCM encryption, ApprovalRequirement::Always prevents LLM self-approval, SkillSource::Synthesized forces Installed trust level.
  • Code quality: No .unwrap() in production code, proper thiserror types, good unit test coverage (46+ tests).

Critical

C1: profile_clear is N+1 queries
ProfileClearTool::execute() loads all facts then calls remove_fact() one-by-one. With 100 facts (the max), that's 101 DB round-trips. Add a delete_profile_facts_by_category() method to UserProfileStore.

C2: store_fact TOCTOU on max_facts limit
Count check and insert aren't atomic. Concurrent distillation tasks (via tokio::spawn) could race past the limit. Fix with a per-user lock or DB-level constraint.

Important

I1: Session summaries are never populated
upsert_session_summary() exists in both backends but nothing calls it anywhere in the diff. The session_search tool will always return empty results. Either add the summary writer (e.g., on session end/compaction) or remove the tool and note it as "coming in a follow-up."

I2: Dead code from path traversal fix
is_safe_skill_name() and old auto_dir.join(&skill.skill_name) remain after switching to UUID-based directory names. Clean up.

I3: FNV-1a content hash isn't collision-resistant
Used for skill dedup unique index (idx_synthesized_skills_dedup). Two different skill contents could collide and silently fail to record. Replace with SHA-256 (already a transitive dep).

I4: No DB integration tests
Good unit tests but no round-trip tests for the 3 new store traits. Project convention (src/db/CLAUDE.md) calls for integration tests using LibSqlBackend::new_memory().

I5: Unbounded tokio::spawn for distillation
No JoinHandle tracking or cancellation token. Orphaned on agent shutdown (bounded by 120s LLM timeout, but still a leak).

Suggestions

  • Cargo.lock contains unrelated windows-sys version downgrades -- split out or confirm intentional.
  • .gitignore adding docs/plans/ would affect all maintainers -- discuss whether this is desired.
  • Consider splitting: session search (incomplete) could be a separate PR from skill synthesis + profile engine.
  • Add clear_profile(user_id, agent_id) method to UserProfileStore for GDPR "forget me" support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: regular 2-5 merged PRs risk: medium Business logic, config, or moderate-risk modules scope: agent Agent core (agent loop, router, scheduler) scope: channel/cli TUI / CLI channel scope: db/libsql libSQL / Turso backend scope: db/postgres PostgreSQL backend scope: db Database trait / abstraction scope: dependencies Dependency updates scope: llm LLM integration scope: tool/builtin Built-in tools scope: tool Tool infrastructure size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants