Bump docker/login-action from 3 to 4#3
Closed
dependabot[bot] wants to merge 1 commit into
Closed
Conversation
Bumps [docker/login-action](https://github.com/docker/login-action) from 3 to 4. - [Release notes](https://github.com/docker/login-action/releases) - [Commits](docker/login-action@v3...v4) --- updated-dependencies: - dependency-name: docker/login-action dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Contributor
Author
LabelsThe following labels could not be found: Please fix the above issues or remove invalid values from |
Contributor
Author
|
OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting If you change your mind, just re-open this PR and I'll resolve any conflicts on it. |
dependabot
Bot
deleted the
dependabot/github_actions/docker/login-action-4
branch
March 12, 2026 01:59
This was referenced Mar 15, 2026
houko
added a commit
that referenced
this pull request
Mar 18, 2026
Knowledge graph cleanup (#3): - Migration v10: add agent_id column to entities and relations tables - KnowledgeStore: add_entity/add_relation now track agent_id - KnowledgeStore: add delete_by_agent() for per-agent cleanup - reset() now cleans up knowledge graph data for the agent Chat memory indicators (#8): - Store memories_saved/memories_used in chat message objects - Add hasMemoryActivity() to detect proactive memory ops - Add memoryIndicatorForMsg() and memoryDetailItems() for display - Update HTML template to use proactive memory fields Inline edit (#5): - Add startEdit/cancelEdit/saveEdit methods to memory page - Double-click or edit button to inline-edit memory content - Enter to save, Escape to cancel
houko
added a commit
that referenced
this pull request
Mar 18, 2026
* Memory: implement mem0-style proactive memory system Implements issue #1106 with: - ProactiveMemory trait: search(), add(), add_with_level(), get(), list() - ProactiveMemoryHooks: auto_memorize(), auto_retrieve() - Multi-level memory: User, Session, Agent - Hook integration: BeforePromptBuild, AgentLoopEnd - Kernel integration for agent loop memory injection Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: overhaul mem0 proactive memory core with LLM extraction, REST API, and bulk ops - Fix critical issues from PR review: dead hook handlers, double-store, empty return, streaming gap, cross-agent memory leakage - Add LLM-powered MemoryExtractor with robust JSON parser (code blocks, invalid input handling), falls back to rule-based DefaultMemoryExtractor - Add full mem0-style REST API (13 endpoints): search, list, add, update, delete, stats, per-agent operations, level-based cleanup, duplicate detection - Add bulk operations: reset (agent), clear_level (scope), cleanup_expired_sessions - Add semantic store primitives: forget_by_agent, forget_by_scope, forget_older_than, count - Integrate session memory auto-cleanup with configurable TTL (session_ttl_hours) - ProactiveMemoryConfig: add #[serde(default)], session_ttl_hours, config.toml docs - Kernel init: smart extractor selection based on extraction_model config - MemoryLevel: derive Default (clippy), add scope_str() centralizing scope mapping - ProactiveMemory::add() returns Vec<MemoryItem>, uses extractor pipeline - auto_memorize: substring-based dedup replacing exact-match only - stats(): efficient SQL COUNT + config status reporting - OnceLock replaces RwLock for set-once proactive_memory on kernel - Remove ~200 lines dead code (fire-and-forget hooks, unused session capture) - Add 12 new tests across semantic store, proactive store, and LLM parser * feat: add mem0-style memory conflict resolution (ADD/UPDATE/NOOP decision flow) - MemoryAction enum: Add, Update(existing_id), Noop for conflict decisions - MemoryExtractor::decide_action() trait method with default heuristic - LLM-powered decide_action() with dedicated conflict resolution prompt - Rewrite add() and auto_memorize() to use unified decision flow - SemanticStore::update_content() for in-place memory updates - Jaccard text similarity for same-category conflict detection - Fix kernel pm_config borrow error * feat: integrate knowledge graph with proactive memory + relation extraction - RelationTriple type for LLM-extracted entity relationships - ExtractionResult now includes relations alongside memories - LLM prompt extracts both memories and knowledge graph triples - store_relations() writes extracted triples to KnowledgeStore (upsert) - graph_context() queries knowledge graph for entity-relevant relations - format_context_with_query() merges semantic memories + graph context - build_prompt_context_with_memory() uses enriched context - Entity/relation type parsers (person, org, works_at, uses, etc.) - Expose MemorySubstrate::knowledge() for graph access * feat: semantic dedup, version history, knowledge graph tests, history API - find_duplicates() uses Jaccard similarity (>0.5) alongside substring matching - Memory version history: UPDATE preserves previous_content + version_history chain - history() method retrieves version chain for any memory - GET /api/memory/{id}/history endpoint for version history - SemanticStore::get_by_id() for single memory lookup - extract_search_keywords() for broader LIKE matching in decision flow - 10 new tests: dedup NOOP, conflicting UPDATE, version history, knowledge graph relations, semantic find_duplicates, text_similarity, entity/relation type parsing, new extraction format with relations, decision response parsing (ADD/UPDATE/NOOP) * feat: update() preserves history, rule-based relation extraction, session TTL, configurable dedup - update() now uses update_content() with version_history chain instead of delete+re-add - DefaultMemoryExtractor extracts basic relations: prefers, works_at, located_in, is_named - auto_retrieve() runs session TTL cleanup automatically before each retrieval - ProactiveMemoryConfig.duplicate_threshold: configurable similarity threshold (default 0.5) - search() enriches results with knowledge graph context when entities match - extract_after_pattern() / capitalize_first() helpers for rule-based extraction * feat: memory consolidation, graph query optimization, relation dedup, more tests - consolidate(): merge duplicate memory groups, soft-delete older entries - POST /api/memory/agents/{id}/consolidate endpoint - graph_context(): targeted entity lookups instead of loading all relations - extract_entity_candidates(): find proper nouns + "User" for graph queries - store_relations() deduplicates: skip existing (source, type, target) triples - KnowledgeStore::has_relation() for relation existence check - find_duplicates() falls back to semantic store when KV store is empty - DefaultMemoryExtractor: "I use X" / "I'm using X" pattern + uses relation - 7 new tests: update version history, relation extraction, relation dedup, consolidation, entity candidate extraction, "I use" pattern * fix: agent_loop auto_memorize now logs extraction results instead of discarding - Non-streaming and streaming paths both log stored memories + relations count - Relations were already being stored inside auto_memorize() via store_relations() - This was the last integration gap identified in the audit * feat: enhanced rule-based extractor, dashboard Memory tab, auto-consolidation - DefaultMemoryExtractor: 30+ patterns covering preferences, facts, tools, task context; extracts precise phrases instead of whole messages - push_memory() deduplicates within same extraction pass - New patterns: "I'm a/I am a", "we're building", "we're migrating to", "I code in", "our stack includes", "the problem is", "I'm debugging", etc. - Task context extraction (session-level) for debugging/fixing patterns - Dashboard: Memory tab under Agents nav section - memory.js: search, list, delete, version history viewer - webchat.rs: include memory.js in script bundle - Auto-consolidation: runs every 10 auto_memorize calls to merge duplicates - AtomicU32 counter with manual Clone impl for ProactiveMemoryStore * feat: integrate embedding driver for vector similarity search in proactive memory - EmbeddingFn trait in proactive.rs (bridge to avoid circular deps) - EmbeddingBridge wraps runtime EmbeddingDriver → EmbeddingFn - ProactiveMemoryStore.with_embedding() builder method - search() / auto_retrieve(): use cosine similarity when embedding available - add_with_decision(): store memories with embeddings for future vector search - init_proactive_memory_full(): unified init with optional LLM + embedding - init_proactive_memory_with_embedding(): convenience constructor - Kernel passes embedding_driver to proactive memory on init - Automatic fallback to LIKE text matching when no embedding driver * fix: resolve critical issues in proactive memory system - Fix route path conflict: /memory/{memory_id} → /memory/items/{memory_id} - Fix UTF-8 panic in truncate_for_log using char_indices - Replace nightly-only is_multiple_of with modulo operator - Write to KV store on add so get()/list() return results - Use nil UUID for default_user_id instead of unparseable "default" - Remove duplicate cleanup_expired_sessions calls in agent_loop - Send only new turn messages to auto_memorize, not full history - Remove duplicate system prompt in LlmMemoryExtractor requests * fix: resolve remaining issues in proactive memory system - Sync KV store on delete/reset/clear/update/consolidate/cleanup - Preserve embedding on update_content instead of nullifying - Per-agent consolidation counters instead of global atomic - extract_search_keywords returns all significant words joined by % - delete() returns false for non-existent memories - Cap find_duplicates to 100 most recent items - Update tests to verify KV store writes and delete behavior - Register all memory routes in OpenAPI spec with proactive-memory tag - Fix utoipa path param mismatch {agent_id} → {id} - Cap search limit to 100 - Add i18n support to Memory dashboard page (en + zh-CN) * fix: address third-round review issues in proactive memory - Validate level param in memory_clear_level, return 400 for invalid - Add stats_all/list_all/search_all for cross-agent dashboard queries - Resolve real agent_id for delete/update KV cleanup - Add KV store write in add_with_level for stats consistency - Combine double get_by_id into single call to fix TOCTOU race - Add badge-user/badge-session/badge-agent CSS classes - Implement after/before filter in semantic recall SQL - Escape LIKE wildcards in user search queries * fix: use single keyword in extract_search_keywords to avoid escape_like conflict The %‐joined multi-keyword output conflicted with escape_like which escapes % characters. Use the longest keyword instead; decide_action handles semantic dedup regardless. * fix: use semantic store MemoryId as KV key for ID consistency The semantic store generates its own UUID in remember(), while MemoryItem has a separate UUID. Using item.id as the KV key meant delete/update could not find the KV entry by the semantic ID. Now all KV keys use the MemoryId returned by remember(). * feat: enhance proactive memory UX and natural conversation integration - Add memories_saved/memories_used fields to AgentLoopResult and API response - Wire memory metadata through REST, WebSocket, and streaming paths - Improve memory injection prompt: agent uses recalled knowledge naturally without announcing "I remember..." — behaves like someone who knows you - Improve extraction prompt: prioritize communication style, frustrations, work preferences; write memories as natural observations with nuance - Add agent filter dropdown on Memory dashboard page - Add manual memory creation form (content + level) - Add memory hit indicator on chat page (brain emoji badge) - Add i18n keys for all new UI elements (en + zh-CN) * feat: add memory confidence decay, import/export, and conflict detection - Confidence decay: exponential decay based on days since last access, boosted by access frequency. Runs at most once per hour during auto_retrieve. - Import/export: GET /api/memory/agents/{id}/export and POST .../import for memory data migration (JSON format) - Conflict detection: when UPDATE replaces contradictory content, surface MemoryConflict in agent response with old vs new content - Dashboard: yellow conflict banner on chat responses, i18n (en + zh-CN) - Config: confidence_decay_rate (default 0.01/day) in ProactiveMemoryConfig * fix: always enable LLM extraction, fall back to default model Previously, LLM-powered memory extraction only activated when extraction_model was explicitly set in config. Now it defaults to the agent's main model, so extraction works out of the box. Users can still set extraction_model to a cheaper model (e.g., llama-3.3-70b) to reduce cost while keeping an expensive model for agent responses. * feat: add proactive memory settings to dashboard and config template - Add Proactive Memory tab to Settings page with toggles and inputs for auto_memorize, auto_retrieve, extraction_model, session_ttl, confidence_decay_rate, and duplicate_threshold - Expose proactive_memory config in GET /api/config and schema endpoint - Add [proactive_memory] section to config.toml generated by `librefang init` - Add i18n translations for all new settings (en + zh-CN) * fix: expose missing proactive memory config fields (max_retrieve, extraction_threshold, extract_categories) - Add max_retrieve, extraction_threshold, extract_categories to GET /api/config - Add same 3 fields to config/schema endpoint - Add UI inputs for all 3 fields in Settings > Proactive Memory tab - Add i18n translations (en + zh-CN) for the new fields * fix: repair smart quotes in zh-CN.json that broke JSON parsing Lines 756-765 used Unicode smart quotes (U+201C/U+201D) as JSON structural delimiters instead of ASCII double quotes. This caused JSON.parse() to fail, breaking the entire Chinese locale. * feat: expose all KernelConfig fields in GET /api/config and config/schema - get_config now returns all ~50 config sections with proper secret redaction (api_key, shared_secret, credentials_path, oauth client IDs) - config_schema now covers all sections for the dashboard dynamic form - Split giant json!() macros into per-section insert calls to avoid Rust recursion limit on macro expansion * fix: harden proactive memory system — bug fixes, TTL cleanup, pagination, memory cap Core fixes (proactive.rs): - Arc<Mutex> for shared Clone state (consolidation_counters, last_decay_run) - Ownership checks on delete/update operations - KV errors logged via tracing::warn instead of silently swallowed - Consolidation counter cleanup after threshold to prevent unbounded growth - TOCTOU race fix in confidence decay scheduling - text_similarity("","") returns 0.0 instead of 1.0 - Import deduplication (similarity > 0.9 skipped) Runtime fixes (agent_loop.rs, proactive_memory.rs): - Deduplicate proactive vs regular recall results - Filter system messages from LLM extraction input - Malformed JSON defaults to NOOP instead of ADD - UPDATE with invalid ID falls back to ADD instead of blindly updating first match - Robust code block stripping (case-insensitive, handles leading text) - stable_prefix_mode skips proactive recall for cache stability API fixes (memory.rs, openapi.rs): - POST /api/memory returns 201 Created - PUT /api/memory/items/{id} validates non-empty content - Memory list endpoint supports pagination (offset/limit) - Missing export/import endpoints added to OpenAPI spec - Restore ProactiveMemory trait import (was incorrectly removed) New features: - Session TTL cleanup: auto-expires session memories older than session_ttl_hours, rate-limited to once per hour, also exposed as POST /api/memory/cleanup - Per-agent memory cap (default 1000): evicts lowest-confidence memories when exceeded - Dashboard: paginated memory list, numeric input validation, removed non-functional level dropdown from add form, max_memories_per_agent setting exposed * fix: address logic review findings — pagination, eviction, cleanup, validation Bug 1: Agent-filtered pagination was broken — added GET /api/memory/agents/{id} list endpoint with offset/limit/total, frontend now uses it instead of abusing the search endpoint with empty query. Bug 2: Over-eviction on batch add — moved evict_if_over_cap to after the ADD/NOOP decision loop so NOOPs don't cause unnecessary eviction. Bug 3: UPDATE fallback creates duplicates — parse_decision_response now tries interpreting existing_id as a 1-based list index when UUID validation fails, matching the numbered format shown to the LLM. Bug 4: cleanup_expired KV orphans — collect agent_ids BEFORE soft-delete so agents whose only memories were expired sessions still get KV cleanup. Bug 5: Empty string → 0 for threshold fields — added empty-string validation before Number() conversions in settings.js. Bug 6: No integer enforcement — added Number.isInteger() checks for max_retrieve, session_ttl_hours, max_memories_per_agent. Also: evict_if_over_cap caps eviction at min(to_evict, current_count) and warns when new_count alone exceeds the cap. * fix: critical logic fixes — ADD action parsing, memory cap enforcement, locale dedup Critical: - parse_decision_response now case-insensitive (ADD/add/Add all work); previously ADD fell through to NOOP, silently dropping new memories - existing_id handles JSON number values (as_u64 fallback), not just strings - Conversation text capped at 8000 chars for LLM extraction to prevent context overflow and runaway costs - auto_memorize now enforces per-agent memory cap (was bypassed entirely) Logic fixes: - find_duplicates marks seed items as used to prevent multi-group assignment - extract_categories always sent on save (empty array clears; was silently preserving old value when cleared) - Removed duplicate i18n key blocks in en.json and zh-CN.json that caused extractionModelDesc to show wrong description - HTML max_retrieve input max=50 aligned with JS validation max=100 * fix: UTF-8 safe truncation and memory cap on all add paths - String::truncate at 8000 bytes could panic on multi-byte chars (Chinese/emoji); now finds nearest char boundary before truncating - add_with_level() and import_memories() were missing evict_if_over_cap() calls, allowing memory cap to be bypassed via these code paths * fix: improve proactive memory robustness and performance - Replace unwrap() with let-else in consolidation to prevent panic - Add count_by_category() SQL method to avoid loading all items for stats - Add migration v9 with composite indexes for recall/decay/eviction queries - Extract maybe_run_maintenance() called from search/consolidate/auto_retrieve - Implement metadata filtering via json_extract in semantic recall - Sanitize error responses to prevent internal info leakage (20 endpoints) - Cap consolidation_counters HashMap to prevent unbounded growth - Log JSON serialization failures during export instead of silent swallow - Fix frontend pagination: reset offset on error, clamp after load * fix: correct SQL alias in WHERE clause and sanitize metadata keys - Replace column alias 'cat' in WHERE with full json_extract() expression (SQLite does not support aliases in WHERE clauses) - Restrict metadata filter keys to alphanumeric/underscore to prevent SQL injection via crafted JSON paths * fix: clarify consolidation winner selection comment * fix: critical bugs found in full PR review - Add 'memory' to validPages and remove redirect to 'sessions' in app.js (memory page was completely unreachable from UI navigation) - Add Array arm to json_to_toml_value so extract_categories saves correctly (was serialized as string "[...]" instead of TOML array) - Filter empty extraction_model string as None to fall back to default model (empty string from settings UI caused LLM API failures) * fix: align CLI template defaults and settings.js with actual config defaults - CLI template: duplicate_threshold 0.85→0.5, confidence_decay_rate 0.05→0.01 - settings.js: max_retrieve default 5→10 to match ProactiveMemoryConfig::default() * fix: address proactive memory logic gaps - Clean up proactive memories when agent is killed (kernel.rs) - Fix KV store ID mismatch by syncing with semantic store ID (proactive.rs) - Re-embed content on memory update so vector search stays accurate (proactive.rs) * feat: hot-reload proactive memory config at runtime - Add UpdateProactiveMemory hot action to config reload system - Change ProactiveMemoryStore.config to Arc<RwLock<>> for runtime updates - Add update_config() method for hot-swapping config without restart - Wire reload through kernel apply_hot_actions * feat: add master enabled toggle for proactive memory - Add enabled field to ProactiveMemoryConfig (defaults to true) - Skip proactive memory init when disabled in kernel boot - Check enabled flag in auto_memorize and auto_retrieve hooks - Expose toggle in config API, schema, settings UI, and CLI template - Fix max_retrieve default from 5 to 10 in CLI templates * fix: include enabled field in MemoryStats API response * feat: add consolidate and cleanup buttons to memory page - Add consolidateAgent() and cleanupExpired() actions to memory.js - Add Consolidate and Cleanup Expired buttons in memory page toolbar - Consolidate button only shows when an agent is selected * feat: add bulk delete for memories - Add POST /api/memory/bulk-delete endpoint accepting { ids: [...] } - Register route in server.rs - Add selection checkboxes to memory table with select-all header - Add bulk delete button (shown when items selected) * fix: clear selection when reloading memory list * feat: knowledge graph cleanup, chat memory indicators, inline edit Knowledge graph cleanup (#3): - Migration v10: add agent_id column to entities and relations tables - KnowledgeStore: add_entity/add_relation now track agent_id - KnowledgeStore: add delete_by_agent() for per-agent cleanup - reset() now cleans up knowledge graph data for the agent Chat memory indicators (#8): - Store memories_saved/memories_used in chat message objects - Add hasMemoryActivity() to detect proactive memory ops - Add memoryIndicatorForMsg() and memoryDetailItems() for display - Update HTML template to use proactive memory fields Inline edit (#5): - Add startEdit/cancelEdit/saveEdit methods to memory page - Double-click or edit button to inline-edit memory content - Enter to save, Escape to cancel * feat: vector cosine similarity dedup + VectorStore trait for backend extensibility - Upgrade find_duplicates() to use vector cosine similarity when stored embeddings are available (mem0-quality dedup), falling back to Jaccard word overlap when no embeddings exist - Enhance default decide_action() heuristic with tiered similarity: vector cosine > substring containment > Jaccard, including high-similarity NOOP detection (≥0.95) and best-candidate UPDATE selection - Add VectorStore trait in librefang-types — backend-agnostic abstraction enabling pluggable vector databases (Qdrant, Pinecone, Chroma, PgVector) - Implement SqliteVectorStore as the default backend with BLOB embeddings and in-process cosine similarity re-ranking - Add cosine_similarity() to types crate as shared utility - Add SemanticStore::get_embeddings_batch() for efficient bulk embedding load - Stash query embedding in _embedding metadata key during decision flow so default heuristic can leverage it without breaking storage paths * feat: add missing API endpoints for full memory management coverage - POST /api/memory/decay — manual confidence decay trigger - GET /api/memory/agents/{id}/count?level= — per-agent memory count - POST /api/memory/agents/{id}/relations — store knowledge graph triples - GET /api/memory/agents/{id}/relations — query knowledge graph - Add ProactiveMemoryStore::query_relations() wrapper for graph queries - Add Knowledge Graph UI panel in memory page (toggle Relations button) - Add Decay Confidence button to memory toolbar - Add loadRelations() and decayConfidence() JS functions --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
houko
added a commit
that referenced
this pull request
Mar 18, 2026
* Memory: implement mem0-style proactive memory system Implements issue #1106 with: - ProactiveMemory trait: search(), add(), add_with_level(), get(), list() - ProactiveMemoryHooks: auto_memorize(), auto_retrieve() - Multi-level memory: User, Session, Agent - Hook integration: BeforePromptBuild, AgentLoopEnd - Kernel integration for agent loop memory injection Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: overhaul mem0 proactive memory core with LLM extraction, REST API, and bulk ops - Fix critical issues from PR review: dead hook handlers, double-store, empty return, streaming gap, cross-agent memory leakage - Add LLM-powered MemoryExtractor with robust JSON parser (code blocks, invalid input handling), falls back to rule-based DefaultMemoryExtractor - Add full mem0-style REST API (13 endpoints): search, list, add, update, delete, stats, per-agent operations, level-based cleanup, duplicate detection - Add bulk operations: reset (agent), clear_level (scope), cleanup_expired_sessions - Add semantic store primitives: forget_by_agent, forget_by_scope, forget_older_than, count - Integrate session memory auto-cleanup with configurable TTL (session_ttl_hours) - ProactiveMemoryConfig: add #[serde(default)], session_ttl_hours, config.toml docs - Kernel init: smart extractor selection based on extraction_model config - MemoryLevel: derive Default (clippy), add scope_str() centralizing scope mapping - ProactiveMemory::add() returns Vec<MemoryItem>, uses extractor pipeline - auto_memorize: substring-based dedup replacing exact-match only - stats(): efficient SQL COUNT + config status reporting - OnceLock replaces RwLock for set-once proactive_memory on kernel - Remove ~200 lines dead code (fire-and-forget hooks, unused session capture) - Add 12 new tests across semantic store, proactive store, and LLM parser * feat: add mem0-style memory conflict resolution (ADD/UPDATE/NOOP decision flow) - MemoryAction enum: Add, Update(existing_id), Noop for conflict decisions - MemoryExtractor::decide_action() trait method with default heuristic - LLM-powered decide_action() with dedicated conflict resolution prompt - Rewrite add() and auto_memorize() to use unified decision flow - SemanticStore::update_content() for in-place memory updates - Jaccard text similarity for same-category conflict detection - Fix kernel pm_config borrow error * feat: integrate knowledge graph with proactive memory + relation extraction - RelationTriple type for LLM-extracted entity relationships - ExtractionResult now includes relations alongside memories - LLM prompt extracts both memories and knowledge graph triples - store_relations() writes extracted triples to KnowledgeStore (upsert) - graph_context() queries knowledge graph for entity-relevant relations - format_context_with_query() merges semantic memories + graph context - build_prompt_context_with_memory() uses enriched context - Entity/relation type parsers (person, org, works_at, uses, etc.) - Expose MemorySubstrate::knowledge() for graph access * feat: semantic dedup, version history, knowledge graph tests, history API - find_duplicates() uses Jaccard similarity (>0.5) alongside substring matching - Memory version history: UPDATE preserves previous_content + version_history chain - history() method retrieves version chain for any memory - GET /api/memory/{id}/history endpoint for version history - SemanticStore::get_by_id() for single memory lookup - extract_search_keywords() for broader LIKE matching in decision flow - 10 new tests: dedup NOOP, conflicting UPDATE, version history, knowledge graph relations, semantic find_duplicates, text_similarity, entity/relation type parsing, new extraction format with relations, decision response parsing (ADD/UPDATE/NOOP) * feat: update() preserves history, rule-based relation extraction, session TTL, configurable dedup - update() now uses update_content() with version_history chain instead of delete+re-add - DefaultMemoryExtractor extracts basic relations: prefers, works_at, located_in, is_named - auto_retrieve() runs session TTL cleanup automatically before each retrieval - ProactiveMemoryConfig.duplicate_threshold: configurable similarity threshold (default 0.5) - search() enriches results with knowledge graph context when entities match - extract_after_pattern() / capitalize_first() helpers for rule-based extraction * feat: memory consolidation, graph query optimization, relation dedup, more tests - consolidate(): merge duplicate memory groups, soft-delete older entries - POST /api/memory/agents/{id}/consolidate endpoint - graph_context(): targeted entity lookups instead of loading all relations - extract_entity_candidates(): find proper nouns + "User" for graph queries - store_relations() deduplicates: skip existing (source, type, target) triples - KnowledgeStore::has_relation() for relation existence check - find_duplicates() falls back to semantic store when KV store is empty - DefaultMemoryExtractor: "I use X" / "I'm using X" pattern + uses relation - 7 new tests: update version history, relation extraction, relation dedup, consolidation, entity candidate extraction, "I use" pattern * fix: agent_loop auto_memorize now logs extraction results instead of discarding - Non-streaming and streaming paths both log stored memories + relations count - Relations were already being stored inside auto_memorize() via store_relations() - This was the last integration gap identified in the audit * feat: enhanced rule-based extractor, dashboard Memory tab, auto-consolidation - DefaultMemoryExtractor: 30+ patterns covering preferences, facts, tools, task context; extracts precise phrases instead of whole messages - push_memory() deduplicates within same extraction pass - New patterns: "I'm a/I am a", "we're building", "we're migrating to", "I code in", "our stack includes", "the problem is", "I'm debugging", etc. - Task context extraction (session-level) for debugging/fixing patterns - Dashboard: Memory tab under Agents nav section - memory.js: search, list, delete, version history viewer - webchat.rs: include memory.js in script bundle - Auto-consolidation: runs every 10 auto_memorize calls to merge duplicates - AtomicU32 counter with manual Clone impl for ProactiveMemoryStore * feat: integrate embedding driver for vector similarity search in proactive memory - EmbeddingFn trait in proactive.rs (bridge to avoid circular deps) - EmbeddingBridge wraps runtime EmbeddingDriver → EmbeddingFn - ProactiveMemoryStore.with_embedding() builder method - search() / auto_retrieve(): use cosine similarity when embedding available - add_with_decision(): store memories with embeddings for future vector search - init_proactive_memory_full(): unified init with optional LLM + embedding - init_proactive_memory_with_embedding(): convenience constructor - Kernel passes embedding_driver to proactive memory on init - Automatic fallback to LIKE text matching when no embedding driver * fix: resolve critical issues in proactive memory system - Fix route path conflict: /memory/{memory_id} → /memory/items/{memory_id} - Fix UTF-8 panic in truncate_for_log using char_indices - Replace nightly-only is_multiple_of with modulo operator - Write to KV store on add so get()/list() return results - Use nil UUID for default_user_id instead of unparseable "default" - Remove duplicate cleanup_expired_sessions calls in agent_loop - Send only new turn messages to auto_memorize, not full history - Remove duplicate system prompt in LlmMemoryExtractor requests * fix: resolve remaining issues in proactive memory system - Sync KV store on delete/reset/clear/update/consolidate/cleanup - Preserve embedding on update_content instead of nullifying - Per-agent consolidation counters instead of global atomic - extract_search_keywords returns all significant words joined by % - delete() returns false for non-existent memories - Cap find_duplicates to 100 most recent items - Update tests to verify KV store writes and delete behavior - Register all memory routes in OpenAPI spec with proactive-memory tag - Fix utoipa path param mismatch {agent_id} → {id} - Cap search limit to 100 - Add i18n support to Memory dashboard page (en + zh-CN) * fix: address third-round review issues in proactive memory - Validate level param in memory_clear_level, return 400 for invalid - Add stats_all/list_all/search_all for cross-agent dashboard queries - Resolve real agent_id for delete/update KV cleanup - Add KV store write in add_with_level for stats consistency - Combine double get_by_id into single call to fix TOCTOU race - Add badge-user/badge-session/badge-agent CSS classes - Implement after/before filter in semantic recall SQL - Escape LIKE wildcards in user search queries * fix: use single keyword in extract_search_keywords to avoid escape_like conflict The %‐joined multi-keyword output conflicted with escape_like which escapes % characters. Use the longest keyword instead; decide_action handles semantic dedup regardless. * fix: use semantic store MemoryId as KV key for ID consistency The semantic store generates its own UUID in remember(), while MemoryItem has a separate UUID. Using item.id as the KV key meant delete/update could not find the KV entry by the semantic ID. Now all KV keys use the MemoryId returned by remember(). * feat: enhance proactive memory UX and natural conversation integration - Add memories_saved/memories_used fields to AgentLoopResult and API response - Wire memory metadata through REST, WebSocket, and streaming paths - Improve memory injection prompt: agent uses recalled knowledge naturally without announcing "I remember..." — behaves like someone who knows you - Improve extraction prompt: prioritize communication style, frustrations, work preferences; write memories as natural observations with nuance - Add agent filter dropdown on Memory dashboard page - Add manual memory creation form (content + level) - Add memory hit indicator on chat page (brain emoji badge) - Add i18n keys for all new UI elements (en + zh-CN) * feat: add memory confidence decay, import/export, and conflict detection - Confidence decay: exponential decay based on days since last access, boosted by access frequency. Runs at most once per hour during auto_retrieve. - Import/export: GET /api/memory/agents/{id}/export and POST .../import for memory data migration (JSON format) - Conflict detection: when UPDATE replaces contradictory content, surface MemoryConflict in agent response with old vs new content - Dashboard: yellow conflict banner on chat responses, i18n (en + zh-CN) - Config: confidence_decay_rate (default 0.01/day) in ProactiveMemoryConfig * fix: always enable LLM extraction, fall back to default model Previously, LLM-powered memory extraction only activated when extraction_model was explicitly set in config. Now it defaults to the agent's main model, so extraction works out of the box. Users can still set extraction_model to a cheaper model (e.g., llama-3.3-70b) to reduce cost while keeping an expensive model for agent responses. * feat: add proactive memory settings to dashboard and config template - Add Proactive Memory tab to Settings page with toggles and inputs for auto_memorize, auto_retrieve, extraction_model, session_ttl, confidence_decay_rate, and duplicate_threshold - Expose proactive_memory config in GET /api/config and schema endpoint - Add [proactive_memory] section to config.toml generated by `librefang init` - Add i18n translations for all new settings (en + zh-CN) * fix: expose missing proactive memory config fields (max_retrieve, extraction_threshold, extract_categories) - Add max_retrieve, extraction_threshold, extract_categories to GET /api/config - Add same 3 fields to config/schema endpoint - Add UI inputs for all 3 fields in Settings > Proactive Memory tab - Add i18n translations (en + zh-CN) for the new fields * fix: repair smart quotes in zh-CN.json that broke JSON parsing Lines 756-765 used Unicode smart quotes (U+201C/U+201D) as JSON structural delimiters instead of ASCII double quotes. This caused JSON.parse() to fail, breaking the entire Chinese locale. * feat: expose all KernelConfig fields in GET /api/config and config/schema - get_config now returns all ~50 config sections with proper secret redaction (api_key, shared_secret, credentials_path, oauth client IDs) - config_schema now covers all sections for the dashboard dynamic form - Split giant json!() macros into per-section insert calls to avoid Rust recursion limit on macro expansion * fix: harden proactive memory system — bug fixes, TTL cleanup, pagination, memory cap Core fixes (proactive.rs): - Arc<Mutex> for shared Clone state (consolidation_counters, last_decay_run) - Ownership checks on delete/update operations - KV errors logged via tracing::warn instead of silently swallowed - Consolidation counter cleanup after threshold to prevent unbounded growth - TOCTOU race fix in confidence decay scheduling - text_similarity("","") returns 0.0 instead of 1.0 - Import deduplication (similarity > 0.9 skipped) Runtime fixes (agent_loop.rs, proactive_memory.rs): - Deduplicate proactive vs regular recall results - Filter system messages from LLM extraction input - Malformed JSON defaults to NOOP instead of ADD - UPDATE with invalid ID falls back to ADD instead of blindly updating first match - Robust code block stripping (case-insensitive, handles leading text) - stable_prefix_mode skips proactive recall for cache stability API fixes (memory.rs, openapi.rs): - POST /api/memory returns 201 Created - PUT /api/memory/items/{id} validates non-empty content - Memory list endpoint supports pagination (offset/limit) - Missing export/import endpoints added to OpenAPI spec - Restore ProactiveMemory trait import (was incorrectly removed) New features: - Session TTL cleanup: auto-expires session memories older than session_ttl_hours, rate-limited to once per hour, also exposed as POST /api/memory/cleanup - Per-agent memory cap (default 1000): evicts lowest-confidence memories when exceeded - Dashboard: paginated memory list, numeric input validation, removed non-functional level dropdown from add form, max_memories_per_agent setting exposed * fix: address logic review findings — pagination, eviction, cleanup, validation Bug 1: Agent-filtered pagination was broken — added GET /api/memory/agents/{id} list endpoint with offset/limit/total, frontend now uses it instead of abusing the search endpoint with empty query. Bug 2: Over-eviction on batch add — moved evict_if_over_cap to after the ADD/NOOP decision loop so NOOPs don't cause unnecessary eviction. Bug 3: UPDATE fallback creates duplicates — parse_decision_response now tries interpreting existing_id as a 1-based list index when UUID validation fails, matching the numbered format shown to the LLM. Bug 4: cleanup_expired KV orphans — collect agent_ids BEFORE soft-delete so agents whose only memories were expired sessions still get KV cleanup. Bug 5: Empty string → 0 for threshold fields — added empty-string validation before Number() conversions in settings.js. Bug 6: No integer enforcement — added Number.isInteger() checks for max_retrieve, session_ttl_hours, max_memories_per_agent. Also: evict_if_over_cap caps eviction at min(to_evict, current_count) and warns when new_count alone exceeds the cap. * fix: critical logic fixes — ADD action parsing, memory cap enforcement, locale dedup Critical: - parse_decision_response now case-insensitive (ADD/add/Add all work); previously ADD fell through to NOOP, silently dropping new memories - existing_id handles JSON number values (as_u64 fallback), not just strings - Conversation text capped at 8000 chars for LLM extraction to prevent context overflow and runaway costs - auto_memorize now enforces per-agent memory cap (was bypassed entirely) Logic fixes: - find_duplicates marks seed items as used to prevent multi-group assignment - extract_categories always sent on save (empty array clears; was silently preserving old value when cleared) - Removed duplicate i18n key blocks in en.json and zh-CN.json that caused extractionModelDesc to show wrong description - HTML max_retrieve input max=50 aligned with JS validation max=100 * fix: UTF-8 safe truncation and memory cap on all add paths - String::truncate at 8000 bytes could panic on multi-byte chars (Chinese/emoji); now finds nearest char boundary before truncating - add_with_level() and import_memories() were missing evict_if_over_cap() calls, allowing memory cap to be bypassed via these code paths * fix: improve proactive memory robustness and performance - Replace unwrap() with let-else in consolidation to prevent panic - Add count_by_category() SQL method to avoid loading all items for stats - Add migration v9 with composite indexes for recall/decay/eviction queries - Extract maybe_run_maintenance() called from search/consolidate/auto_retrieve - Implement metadata filtering via json_extract in semantic recall - Sanitize error responses to prevent internal info leakage (20 endpoints) - Cap consolidation_counters HashMap to prevent unbounded growth - Log JSON serialization failures during export instead of silent swallow - Fix frontend pagination: reset offset on error, clamp after load * fix: correct SQL alias in WHERE clause and sanitize metadata keys - Replace column alias 'cat' in WHERE with full json_extract() expression (SQLite does not support aliases in WHERE clauses) - Restrict metadata filter keys to alphanumeric/underscore to prevent SQL injection via crafted JSON paths * fix: clarify consolidation winner selection comment * fix: critical bugs found in full PR review - Add 'memory' to validPages and remove redirect to 'sessions' in app.js (memory page was completely unreachable from UI navigation) - Add Array arm to json_to_toml_value so extract_categories saves correctly (was serialized as string "[...]" instead of TOML array) - Filter empty extraction_model string as None to fall back to default model (empty string from settings UI caused LLM API failures) * fix: align CLI template defaults and settings.js with actual config defaults - CLI template: duplicate_threshold 0.85→0.5, confidence_decay_rate 0.05→0.01 - settings.js: max_retrieve default 5→10 to match ProactiveMemoryConfig::default() * fix: address proactive memory logic gaps - Clean up proactive memories when agent is killed (kernel.rs) - Fix KV store ID mismatch by syncing with semantic store ID (proactive.rs) - Re-embed content on memory update so vector search stays accurate (proactive.rs) * feat: hot-reload proactive memory config at runtime - Add UpdateProactiveMemory hot action to config reload system - Change ProactiveMemoryStore.config to Arc<RwLock<>> for runtime updates - Add update_config() method for hot-swapping config without restart - Wire reload through kernel apply_hot_actions * feat: add master enabled toggle for proactive memory - Add enabled field to ProactiveMemoryConfig (defaults to true) - Skip proactive memory init when disabled in kernel boot - Check enabled flag in auto_memorize and auto_retrieve hooks - Expose toggle in config API, schema, settings UI, and CLI template - Fix max_retrieve default from 5 to 10 in CLI templates * fix: include enabled field in MemoryStats API response * feat: add consolidate and cleanup buttons to memory page - Add consolidateAgent() and cleanupExpired() actions to memory.js - Add Consolidate and Cleanup Expired buttons in memory page toolbar - Consolidate button only shows when an agent is selected * feat: add bulk delete for memories - Add POST /api/memory/bulk-delete endpoint accepting { ids: [...] } - Register route in server.rs - Add selection checkboxes to memory table with select-all header - Add bulk delete button (shown when items selected) * fix: clear selection when reloading memory list * feat: knowledge graph cleanup, chat memory indicators, inline edit Knowledge graph cleanup (#3): - Migration v10: add agent_id column to entities and relations tables - KnowledgeStore: add_entity/add_relation now track agent_id - KnowledgeStore: add delete_by_agent() for per-agent cleanup - reset() now cleans up knowledge graph data for the agent Chat memory indicators (#8): - Store memories_saved/memories_used in chat message objects - Add hasMemoryActivity() to detect proactive memory ops - Add memoryIndicatorForMsg() and memoryDetailItems() for display - Update HTML template to use proactive memory fields Inline edit (#5): - Add startEdit/cancelEdit/saveEdit methods to memory page - Double-click or edit button to inline-edit memory content - Enter to save, Escape to cancel * feat: vector cosine similarity dedup + VectorStore trait for backend extensibility - Upgrade find_duplicates() to use vector cosine similarity when stored embeddings are available (mem0-quality dedup), falling back to Jaccard word overlap when no embeddings exist - Enhance default decide_action() heuristic with tiered similarity: vector cosine > substring containment > Jaccard, including high-similarity NOOP detection (≥0.95) and best-candidate UPDATE selection - Add VectorStore trait in librefang-types — backend-agnostic abstraction enabling pluggable vector databases (Qdrant, Pinecone, Chroma, PgVector) - Implement SqliteVectorStore as the default backend with BLOB embeddings and in-process cosine similarity re-ranking - Add cosine_similarity() to types crate as shared utility - Add SemanticStore::get_embeddings_batch() for efficient bulk embedding load - Stash query embedding in _embedding metadata key during decision flow so default heuristic can leverage it without breaking storage paths * feat: add missing API endpoints for full memory management coverage - POST /api/memory/decay — manual confidence decay trigger - GET /api/memory/agents/{id}/count?level= — per-agent memory count - POST /api/memory/agents/{id}/relations — store knowledge graph triples - GET /api/memory/agents/{id}/relations — query knowledge graph - Add ProactiveMemoryStore::query_relations() wrapper for graph queries - Add Knowledge Graph UI panel in memory page (toggle Relations button) - Add Decay Confidence button to memory toolbar - Add loadRelations() and decayConfidence() JS functions --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This was referenced Mar 20, 2026
This was referenced Apr 5, 2026
houko
added a commit
that referenced
this pull request
Apr 14, 2026
Three real issues found while reviewing the original commit on this branch: 1. **`/api/status` still reachable.** The status handler at routes/config.rs:48 returns the full agents listing (id, name, state, model provider, profile) plus `home_dir`, `api_listen`, session count and memory usage — exactly the enumeration surface `require_auth_for_reads` exists to close. The original commit kept it in `always_public_method_free`, so flipping the flag did not actually stop `/api/status` from leaking. Moved it into `dashboard_read_exact` (still GET-only) so the flag locks it down. 2. **Flag silently no-ops when only user_api_keys or dashboard credentials are configured.** The gate was `require_auth_for_reads && api_key_present` where `api_key_present = !api_key.trim().is_empty()`. Production deployments commonly configure per-user keys or dashboard user/pass without a standalone api_key — in those cases, flipping the flag did nothing. Replaced with the full "any auth configured" predicate: `api_key || !user_api_keys.is_empty() || dashboard_auth_enabled`, which mirrors the existing auth-bypass check a few lines below. 3. **No operator feedback when flag is on but no auth exists.** If `require_auth_for_reads = true` is set without any auth configured, the flag is (correctly) a no-op at the middleware layer — but the operator gets no signal. Added a startup `warn!` in `build_router` so the misconfig is visible in logs at boot. Tests added: - `test_require_auth_for_reads_blocks_api_status` — locks in the fix for #1. - `test_require_auth_for_reads_engages_with_user_api_keys_only` — locks in the fix for #2 (both the 401 on missing creds and the 200 on a valid per-user key). - `test_require_auth_for_reads_is_noop_without_any_auth` — pins the middleware contract for #3 (the warning handles UX, middleware stays permissive when no auth backends exist). Verified: `cargo test -p librefang-api --lib middleware` — 20 passed (7 new for this feature) and `cargo clippy -p librefang-api -p librefang-types --all-targets -- -D warnings` clean.
3 tasks
houko
added a commit
that referenced
this pull request
Apr 14, 2026
…2398) * feat(api): add require_auth_for_reads flag to lock down dashboard reads The dashboard public-read allowlist in the auth middleware hard-codes /api/agents, /api/config, /api/budget, /api/sessions, /api/approvals, /api/hands, /api/skills, /api/workflows and more as GET-public so the SPA can render before the user enters credentials. With the default api_listen = "0.0.0.0:4545", any host on the LAN can enumerate agents, read configuration (minus redacted api_key), observe spend, and list pending approvals without a token — a hard mismatch with the "Bearer token authentication" line in SECURITY.md. Introduce KernelConfig.require_auth_for_reads (default false, so existing deployments keep rendering unchanged). When it is true AND api_key is configured, the middleware collapses the allowlist to the static-asset / OAuth / health subset and forces every dashboard read through bearer authentication. Unauthenticated health probes, OAuth callback, and dashboard shell HTML remain reachable. Refactor the public-path match into matches!()-based groups so clippy (nonminimal_bool) stays quiet and the two tiers are obviously separated in code review. Regression coverage: - require_auth_for_reads=true blocks unauthenticated GET /api/agents - require_auth_for_reads=true still allows the correct bearer - /api/health stays public with the flag on - require_auth_for_reads=false preserves the legacy public GET * fix(api): close require_auth_for_reads gaps found in self-review Three real issues found while reviewing the original commit on this branch: 1. **`/api/status` still reachable.** The status handler at routes/config.rs:48 returns the full agents listing (id, name, state, model provider, profile) plus `home_dir`, `api_listen`, session count and memory usage — exactly the enumeration surface `require_auth_for_reads` exists to close. The original commit kept it in `always_public_method_free`, so flipping the flag did not actually stop `/api/status` from leaking. Moved it into `dashboard_read_exact` (still GET-only) so the flag locks it down. 2. **Flag silently no-ops when only user_api_keys or dashboard credentials are configured.** The gate was `require_auth_for_reads && api_key_present` where `api_key_present = !api_key.trim().is_empty()`. Production deployments commonly configure per-user keys or dashboard user/pass without a standalone api_key — in those cases, flipping the flag did nothing. Replaced with the full "any auth configured" predicate: `api_key || !user_api_keys.is_empty() || dashboard_auth_enabled`, which mirrors the existing auth-bypass check a few lines below. 3. **No operator feedback when flag is on but no auth exists.** If `require_auth_for_reads = true` is set without any auth configured, the flag is (correctly) a no-op at the middleware layer — but the operator gets no signal. Added a startup `warn!` in `build_router` so the misconfig is visible in logs at boot. Tests added: - `test_require_auth_for_reads_blocks_api_status` — locks in the fix for #1. - `test_require_auth_for_reads_engages_with_user_api_keys_only` — locks in the fix for #2 (both the 401 on missing creds and the 200 on a valid per-user key). - `test_require_auth_for_reads_is_noop_without_any_auth` — pins the middleware contract for #3 (the warning handles UX, middleware stays permissive when no auth backends exist). Verified: `cargo test -p librefang-api --lib middleware` — 20 passed (7 new for this feature) and `cargo clippy -p librefang-api -p librefang-types --all-targets -- -D warnings` clean. * fix(api): also lock /api/health/detail behind require_auth_for_reads Follow-up finding from self-review. `/api/health/detail`'s own handler doc comment at routes/config.rs:317 says "requires auth", but the middleware allowlist had it in the always-public set. The handler returns operational data that should not be reachable from a cold probe: - `panic_count` / `restart_count` from the supervisor - `agent_count` - `embedding_provider` / `embedding_model` / `extraction_model` (infra leak) - `config_warnings` — the full output of `KernelConfig::validate()`, which can tell a remote attacker exactly what's wrong with the deployment - event-bus `dropped_events` count Moved to `dashboard_read_exact` so it gets locked down when the flag is on. `/api/health` stays public because its payload is genuinely minimal (`status`, `version`, two-item `checks` array) and load balancers / orchestrators need it for probing. Test added: `test_require_auth_for_reads_blocks_api_health_detail` — pins both contracts (/api/health stays public, /api/health/detail becomes auth-required) in a single test. Note: this is a partial fix. With the flag OFF, `/api/health/detail` stays public to preserve backwards compatibility, which means the handler's own "requires auth" doc comment is still being violated in the default configuration. Making it always-require-auth is a separate behavioural change that belongs in its own PR. Tests: `cargo test -p librefang-api --lib middleware` — 21 passed (8 for require_auth_for_reads, up from 7). * fix(api): close residual info leaks in unauthenticated endpoints Two more findings from self-review of the always-public set: 1. **`/api/auth/dashboard-check` echoed the configured dashboard username to anonymous callers.** The SPA uses this endpoint before the user has logged in (to pick the right login form), so the route is legitimately unauthenticated — but returning `"username": "<admin>"` handed an anonymous remote caller one half of the credential pair, enabling targeted credential stuffing against `/auth/dashboard-login`. The `mode` field is sufficient for the SPA to pick the right login form; the user already knows their own username. Now always returns an empty string. 2. **`/api/version` echoed the machine hostname.** Version endpoints conventionally expose build info, but the hostname is a per-machine identifier that lets a remote probe correlate a daemon to a specific deployment target. Dropped from the payload. Operators who need the hostname should read it from the daemon's shell environment. No tests referenced either field, `cargo test -p librefang-api --lib` passes with 262 tests (no behaviour change for the dashboard SPA beyond needing the user to type their username at login time, which is how every other dashboard already works). Residual pre-existing gap noted for follow-up: `/api/health/detail`'s own doc comment says it "requires auth" but the middleware kept it public until this PR's flag was added. With the flag off it's still public, honouring backwards compatibility for existing operator probes. Making it unconditionally auth-required is a separate behavioural change. * fix(api): make /api/health/detail always require auth, matching its doc The handler at routes/config.rs:317 documents itself as "Full health diagnostics (requires auth)" and `/api/health`'s doc explicitly says "Use GET /api/health/detail for full diagnostics (requires auth)". The middleware was the only thing still treating it as public — a pre-existing mismatch that this PR's first pass only partially closed by flag-gating. Remove it from both `always_public` and `dashboard_read_exact`. With the endpoint in neither public list, the middleware's default auth-required path handles it, so `/api/health/detail` now requires a bearer token regardless of `require_auth_for_reads`. The handler contract and the middleware contract finally agree. Breaking change for operators: if anyone was probing `/api/health/detail` without auth, they'll start getting 401. `/api/health` (minimal liveness) stays public for load balancers and orchestrators, so the standard deployment probe path is unaffected. Monitoring setups that want the detailed view should configure a bearer token — that was the original design intent. Test replaced: `test_api_health_detail_always_requires_auth` now pins both directions — `/api/health` stays public with the flag off, and `/api/health/detail` is 401 regardless of whether the flag is on or off. Tests: `cargo test -p librefang-api --lib middleware` — 21 passed.
6 tasks
houko
added a commit
that referenced
this pull request
Apr 18, 2026
…ation Fifth Codex batch on #2704 — covers the skill-evolution subsystem (code the upstream PR #2694 shipped to main but that shows up in this PR's diff), plus the KV race on worker telemetry writes. librefang-skills (evolution.rs): - fuzzy_find_and_replace rejects empty old_string up front (#16). - update_skill / patch_skill re-verify skill_dir.exists() under the lock so concurrent delete_skill can't be resurrected (#3, #5). - delete_skill runs validate_name to reject path traversal (#12). - remove_supporting_file canonicalises + contains-checks the target (#6), matching write_supporting_file. - write_supporting_file scans content BEFORE writing (#7) so a rejected update doesn't destroy the pre-existing valid file. librefang-runtime (tool_runner.rs): - tool_skill_evolve_delete resolves the real installed skill's parent dir via registry.get() (#15) instead of unconditionally targeting the global skills_dir. librefang-types / librefang-kernel (approval policy): - Default require_approval list includes skill_evolve_* (#14). Updated serde `true`-shorthand + tests to match. librefang-kernel (kernel/mod.rs): - build_skill_summary sanitises the category key (#1) — tags are third-party data. - extract_json_from_llm_response tries every '{' instead of giving up after the first (#8). - Background skill review uses the agent's resolved driver and model for cost attribution (#13); falls back to defaults only when manifest resolution fails. worker (github-stats-worker/index.js): - Click counts sharded across 8 blobs, unioned on read (#39). - UI error reports sharded across 4 blobs plus legacy-key fallback for historical data (#41).
houko
added a commit
that referenced
this pull request
Apr 20, 2026
…oard parity, smoke script Six follow-ups bundled per request to keep the channel-progress effort in one PR. Done from #2 onward; #1 (per-OutputFormat backtick adaptation) was investigated and dropped — backtick renders correctly in TelegramHtml (<code>), SlackMrkdwn, Discord/Matrix Markdown (inline code), and the only adapter that strips backticks (PlainText for Mastodon) is already opted out of the progress path via suppress_error_responses. #2 buffered_text fallback test: new integration test test_bridge_streaming_adapter_kernel_and_transport_both_fail covers the V2 4th outcome (send_streaming Err + kernel Err). #3 surface context_warning PhaseChange so the user sees when the agent's context window was trimmed/overflowed. Other phases stay SSE-only. #4 collapse repeated tool calls within an iteration — replace last_progress_tool with iter_tools_seen HashSet cleared at every ContentComplete. Batch agents no longer spam "🔧 web_search" per parallel call; retries in a later iteration still get a fresh line. #5 i18n for the "failed" suffix — supports en/zh-CN/es/ja/de/fr (matches librefang_types::i18n). Language threaded from kernel.config_snapshot().language through start_stream_text_bridge. #6 prettify tool names — web_search → Web Search; MCP_call → MCP Call (preserves internal caps). Backticks dropped from progress lines since the prettified form is now the identity. #8 dashboard ToolCallCard parity — mirror prettifyToolName() in dashboard/src/lib/string.ts and apply it in the card header so chat reply and dashboard show the same rendering. #7 live-integration smoke script — scripts/tests/channel_progress_smoke.sh automates the daemon-side flow once the user supplies an LLM API key and a configured channel adapter. Documents the channel-delivery gap (needs an external webhook receiver). No driver injection hook added to the kernel — that would have been scope creep. Tests added/updated: - test_prettify_tool_name_snake_to_title / _kebab_and_dotted / _preserves_internal_caps - test_tr_progress_failed_languages - test_bridge_streaming_adapter_kernel_and_transport_both_fail - Updated existing assertions to expect prettified names
houko
added a commit
that referenced
this pull request
Apr 20, 2026
…rd prettify, smoke script auto-spawn Three minor follow-ups from review: #3 — Symmetric blank lines around progress markers. ToolUseStart used \n\n…\n; ToolExecutionResult and PhaseChange used \n…\n. Adjacent markers (e.g. 🔧 X right before⚠️ X failed) ended up on consecutive lines without a blank separator and many markdown renderers collapsed them into one paragraph. All three now use \n\n…\n\n so every renderer that respects markdown blank-line semantics shows them as separate blocks. #4 — prettifyToolName() in dashboard/src/lib/string.ts now iterates by Unicode codepoint (via spread) instead of UTF-16 unit. A tool name starting with a non-BMP character (e.g. emoji) no longer drops its surrogate half when uppercased. Mirrors the Rust-side prettifier in channel_bridge.rs which uses chars().next() (also codepoint-correct). Tool names are usually ASCII so this is forward-looking, not a fix. #5 — channel_progress_smoke.sh now auto-spawns a temporary agent when the daemon has none. Picks the first available LLM key (GROQ/OPENAI/ANTHROPIC/MINIMAX) for the provider, sends a minimal manifest_toml inline (the SpawnRequest schema requires either manifest_toml or template — name alone isn't valid), and despawns the agent on exit. Pre-existing agents are still reused untouched.
houko
added a commit
that referenced
this pull request
Apr 20, 2026
…m fix, show_progress, i18n, prettify, dashboard parity (#2793) * feat(channels): channel-progress v2 — Telegram fallback fix, show_progress config, integration test Three follow-ups to #2792 (channel progress markers): 1. Telegram (streaming-adapter) buffered_text fallback now uses the `_status` variant. The same regression class fixed for non-streaming adapters in V1 was still latent on the Telegram path: - send_streaming Ok + kernel Err → Done reaction + success=true - send_streaming Err + buffered → Done reaction + success=true Both now correctly emit Error reaction, record_delivery(false), and journal Failed when the kernel actually errored. The fallback path also honors `suppress_error_responses` when the buffered text is a sanitized error (defense-in-depth — Telegram is not in the opt-in list today, but the path is now uniform across all adapters). 2. New `agent.toml show_progress` field (default true) plumbed through the streaming bridge. When false, the bridge skips both the `🔧 tool_name` and `⚠️ tool_name failed` injections — useful for agents whose output is parsed downstream, or for pristine-output scenarios where status markers would leak into the response. The trait-impl side looks up the agent's manifest from the kernel registry once per dispatch and passes it through to `start_stream_text_bridge[_with_status]`. 3. New end-to-end integration test in `crates/librefang-channels/tests/bridge_integration_test.rs` that wires a real `BridgeManager` + `MockAdapter` (non-streaming) + `MockProgressHandle` (override of `send_message_streaming_with_sender_status` that synthesises a delta stream with progress markers). Verifies the V2 dispatch pipeline actually surfaces progress to non-streaming adapters end-to-end via the consolidated `send_response` call. Tests: - test_stream_bridge_show_progress_false_suppresses_all_markers: show_progress=false produces no 🔧/⚠️ markers but still flows the actual model prose through - test_bridge_non_streaming_adapter_sees_progress_markers: full BridgeManager → MockAdapter pipeline; verifies progress markers end up in the captured `send()` call Notes: - True "live daemon" integration test (per CLAUDE.md) requires LLM API keys + a configured channel adapter (Telegram bot token, etc.) which the daemon environment does not currently have. The in-process integration test exercises the full dispatch wiring using real tokio tasks/channels and a mock kernel handle. * fix(kernel/wizard): add show_progress to AgentManifest struct literal CI compile error: wizard.rs constructed AgentManifest with an explicit struct literal listing every field, so adding show_progress in V2 broke that one site (the wizard agent created via setup intent). All other AgentManifest constructors use ..Default::default() and pick up show_progress=true automatically; only this one was a full-literal. * feat(channels): channel-progress v3 — collapse, i18n, prettify, dashboard parity, smoke script Six follow-ups bundled per request to keep the channel-progress effort in one PR. Done from #2 onward; #1 (per-OutputFormat backtick adaptation) was investigated and dropped — backtick renders correctly in TelegramHtml (<code>), SlackMrkdwn, Discord/Matrix Markdown (inline code), and the only adapter that strips backticks (PlainText for Mastodon) is already opted out of the progress path via suppress_error_responses. #2 buffered_text fallback test: new integration test test_bridge_streaming_adapter_kernel_and_transport_both_fail covers the V2 4th outcome (send_streaming Err + kernel Err). #3 surface context_warning PhaseChange so the user sees when the agent's context window was trimmed/overflowed. Other phases stay SSE-only. #4 collapse repeated tool calls within an iteration — replace last_progress_tool with iter_tools_seen HashSet cleared at every ContentComplete. Batch agents no longer spam "🔧 web_search" per parallel call; retries in a later iteration still get a fresh line. #5 i18n for the "failed" suffix — supports en/zh-CN/es/ja/de/fr (matches librefang_types::i18n). Language threaded from kernel.config_snapshot().language through start_stream_text_bridge. #6 prettify tool names — web_search → Web Search; MCP_call → MCP Call (preserves internal caps). Backticks dropped from progress lines since the prettified form is now the identity. #8 dashboard ToolCallCard parity — mirror prettifyToolName() in dashboard/src/lib/string.ts and apply it in the card header so chat reply and dashboard show the same rendering. #7 live-integration smoke script — scripts/tests/channel_progress_smoke.sh automates the daemon-side flow once the user supplies an LLM API key and a configured channel adapter. Documents the channel-delivery gap (needs an external webhook receiver). No driver injection hook added to the kernel — that would have been scope creep. Tests added/updated: - test_prettify_tool_name_snake_to_title / _kebab_and_dotted / _preserves_internal_caps - test_tr_progress_failed_languages - test_bridge_streaming_adapter_kernel_and_transport_both_fail - Updated existing assertions to expect prettified names * fix(channels): correct success/err pairing + treat timeout as soft success + clippy collapsible_if Three review-driven fixes for the V3 PR: Bug 1 — Telegram path outcome 3 (send_streaming Err + kernel Ok) was recording delivery as success=true with err=Some(stream_error). The fallback send_response had already delivered the buffered text and the kernel succeeded, so the transport-side stream error is irrelevant to delivery accounting — keeping it in the err field produced a contradictory metric (success AND err). Now: err is Some only when kernel actually failed. Bug 2 — TIMEOUT_PARTIAL_OUTPUT_MARKER was being mapped to status=Err, which flipped the lifecycle reaction to Error and record_delivery to success=false. Pre-V2 the bridge had no status channel and treated these turns as Done because the model emitted useful partial output before the inactivity timer fired. Restore that semantics: status=Ok for timeouts. The user still sees the "[Task timed out…]" tail appended to their reply. Clippy collapsible_if — flatten the inner if show_progress && ... into match guards on the ToolExecutionResult and PhaseChange arms (CI clippy -D warnings caught this in run 24651228831). * chore(channels): drop dead 'let _ = e' annotation The variable 'e' (stream transport error) is already used at warn! line 2886 and the empty-buffer fallback at line 2953, so the explicit underscore-binding introduced by the Bug 1 fix is noise — clean it up. * test(channels): regression coverage for Bug 1 (success/err pairing) + Bug 2 (timeout-as-success) Both bugs were caught by review, not tests — add the missing coverage so they cannot silently regress. Bug 2 unit test: test_stream_bridge_timeout_partial_output_reports_ok_status Constructs a kernel handle that fails with a string containing TIMEOUT_PARTIAL_OUTPUT_MARKER. Asserts: - The user-facing text channel still receives the "[Task timed out…]" tail so the user knows the reply may be incomplete. - The status oneshot resolves to Ok(()) — NOT Err — so bridge.rs drives the lifecycle reaction to Done and record_delivery to success=true. Pre-V2 semantics preserved. Bug 1 integration test: test_bridge_streaming_adapter_kernel_ok_transport_fail_records_clean_success Adds MockKernelOkHandle (overrides send_message_streaming_with_sender_status to emit clean text + status=Ok, AND overrides record_delivery to capture every (success, err) pair). Combined with the existing MockFailingStreamingAdapter (always Err on send_streaming), the test exercises Telegram outcome 3: - Fallback send_response delivers the buffered text ✓ - record_delivery is called with success=true AND err=None — proving the transport-side stream error does NOT leak into the err field when the kernel itself succeeded. (Pre-fix this would have been success=true, err=Some(stream_e) — a contradictory metric.) * polish(channels): unify progress-line spacing, codepoint-safe dashboard prettify, smoke script auto-spawn Three minor follow-ups from review: #3 — Symmetric blank lines around progress markers. ToolUseStart used \n\n…\n; ToolExecutionResult and PhaseChange used \n…\n. Adjacent markers (e.g. 🔧 X right before⚠️ X failed) ended up on consecutive lines without a blank separator and many markdown renderers collapsed them into one paragraph. All three now use \n\n…\n\n so every renderer that respects markdown blank-line semantics shows them as separate blocks. #4 — prettifyToolName() in dashboard/src/lib/string.ts now iterates by Unicode codepoint (via spread) instead of UTF-16 unit. A tool name starting with a non-BMP character (e.g. emoji) no longer drops its surrogate half when uppercased. Mirrors the Rust-side prettifier in channel_bridge.rs which uses chars().next() (also codepoint-correct). Tool names are usually ASCII so this is forward-looking, not a fix. #5 — channel_progress_smoke.sh now auto-spawns a temporary agent when the daemon has none. Picks the first available LLM key (GROQ/OPENAI/ANTHROPIC/MINIMAX) for the provider, sends a minimal manifest_toml inline (the SpawnRequest schema requires either manifest_toml or template — name alone isn't valid), and despawns the agent on exit. Pre-existing agents are still reused untouched. * fix(channels/tests): extract DeliveryLog type alias to satisfy clippy::type_complexity CI clippy -D warnings rejected Arc<Mutex<Vec<(bool, Option<String>)>>> as too complex. Hoist into a named alias — same shape, named contract.
3 tasks
houko
added a commit
that referenced
this pull request
Apr 25, 2026
…n (refs #3138) (#3162) * fix(dashboard): expose vLLM + LM Studio in embedding provider dropdown (refs #3138) Memory page's embedding provider <select> only listed openai / ollama / gemini / minimax, but the runtime backend (`librefang-runtime/src/embedding.rs`) has supported `vllm` and `lmstudio` for a while — match arms exist, default URLs are hardcoded (`http://127.0.0.1:8000/v1` and `http://127.0.0.1:1234/v1`), and `detect_embedding_provider()` already considers `VLLM_BASE_URL` and `LMSTUDIO_BASE_URL`. The dropdown was the missing piece, leaving self-host users with no way to select either through the UI. Also adds an inline hint under the API Key Env input clarifying that local providers (Ollama / vLLM / LM Studio) typically don't need a key — the placeholder `OPENAI_API_KEY` was misleading for self-host workflows. This addresses the "no `OLLAMA_API_KEY` field equivalent" UX concern in the issue (the field is generic text, not provider-specific; the hint makes that explicit). Refs #3138 sub-items 4 (custom provider not in embedding list) and 6 (no `OLLAMA_API_KEY` field). The other sub-items (OpenAI provider missing from list — #3, embedding list providers broken at runtime — #5) need deeper investigation in the catalog filter / driver layer and are deliberately out of scope for this minimal dashboard fix. Changes: - `crates/librefang-api/dashboard/src/pages/MemoryPage.tsx`: * Add `vllm` and `lmstudio` <option>s. * Add caption under API Key Env input via `t("memory.api_key_env_hint")`. - `crates/librefang-api/dashboard/src/locales/en.json`: * `memory.provider_vllm = "vLLM"` * `memory.provider_lmstudio = "LM Studio"` * `memory.api_key_env_hint` (the new caption). - `zh.json` left unchanged: the existing memory provider options also rely on `defaultValue` fallback for English labels (no `provider_openai` / `provider_ollama` keys in zh memory namespace today), so this PR follows the same pattern. A full zh translation pass is a separate concern. Verification: dashboard typecheck not run locally (no node_modules in fresh worktree); changes are pure JSX option additions + i18n key additions with `defaultValue`, so no new TypeScript types introduced. CI typecheck will gate. * test(api): regenerate kernel_config_schema golden fixture for new embedding providers
houko
added a commit
that referenced
this pull request
Apr 26, 2026
Two follow-up fixes for the granular MCP taint policy review: **Low #3 — JSONPath dotted-key limitation** The pattern parser splits on `.` and treats `[` as the start of array notation, so a JSON object key containing `.` or `[` (e.g. `"a.b"`, `"items[0]"`) cannot be addressed precisely. Quoted-segment syntax (`$.headers."content-type"`) is also not parsed. In practice MCP tool schemas rarely use such keys, but the matcher fails closed (no false positive skip) when it bites. - Add a "Limitation" section to `jsonpath_matches` rustdoc. - New regression test `test_jsonpath_dotted_keys_are_known_limitation` pins the current behaviour so a future change has to opt in. **Low #4 — silent typos in `tool_policy.rule_sets`** `lookup_rule_set_action` previously returned `None` when a referenced name was missing from the registry, so an operator typo (e.g. `"audit_typo"` instead of `"audit"`) became a silent no-op. Now: - New `warn_unknown_rule_set_once(name, tool)` emits a one-shot WARN on first scan that observes a missing name, deduped per process via a static `OnceLock<Mutex<HashSet<String>>>`. - Wired into `lookup_rule_set_action` at the existing skip site. - New `GET /api/mcp/taint-rules` exposes the registered `[[taint_rules]]` names + actions so the dashboard can validate inputs *before* the operator hits Save (separate commit handles the frontend wiring; the endpoint is read-only and reuses the existing auth chain). - Regression test verifies the unknown-name path returns `None` and doesn't panic / corrupt the dedup set on repeated calls.
houko
added a commit
that referenced
this pull request
Apr 26, 2026
#3050) (#3193) * feat(runtime): granular MCP taint policy with rule sets and dashboard tree editor - McpTaintToolPolicy gains a 'default' field (scan | skip) so a single line can bypass scanning for noisy tools (browser tab handles, DB session IDs, etc.) without enumerating every argument path. - Document JSONPath wildcard semantics ($.foo, $.foo.*, $.foo[*], $.*) in McpTaintPathPolicy rustdoc and add a dedicated wildcard test that asserts each documented form against the in-tree matcher. - Introduce top-level [[taint_rules]] with named NamedTaintRuleSet entries (action: block | warn | log, plus a list of TaintRuleId). Tool policies can reference them via 'rule_sets'; the scanner picks the most permissive action across overlapping sets and emits structured tracing events for warn/log so operators can build an exemption baseline before flipping a set to block. - Wire the registry from KernelConfig.taint_rules through every install/reload path in the kernel into the runtime McpServerConfig (taint_rule_sets snapshot) so live taint scans apply the latest rule-set actions. - Surface taint_scanning + taint_policy in /api/mcp/servers and /api/mcp/servers/{name} so the dashboard can hydrate without a second fetch. - Dashboard: add TaintPolicyEditor (server -> tool -> path tree with in-place add/edit/remove, default action selector, rule_sets reference, per-path rule toggles), wire a 'Taint' button into ServerCard, and route saves through useUpdateMcpTaintPolicy which invalidates mcpKeys.servers / mcpKeys.server / mcpKeys.health so the card refreshes without a full config reload. - Add TaintRuleId / McpTaintToolAction / McpTaintRuleSetAction TS types mirroring the snake_case serde tags. - Round-trip serde tests cover legacy configs (no 'default', no 'rule_sets'), new tool-level skip, wildcard paths, and all three rule-set severity tiers. Closes #3050 * fix(runtime-mcp): multi-rule masking + kernel snapshot helper The taint scanner returned only the first rule that fired, so a rule set authorized to downgrade rule A could silently allow rule B through when both fired on the same payload (e.g. a Secret-rule warn downgrade masking a PII-rule fire on `[email protected]`). - Add `detect_outbound_text_violation_rules_with_skip` returning every rule that fires; old `_with_skip` keeps the first-only signature for backward-compat callers. - `walk_taint` now iterates every fired rule and blocks on the first one not downgraded by a rule set. - Regression test asserts the masking case blocks. - Fix pre-existing `tool_policy_rule_sets_reference_round_trips` that used the wrong toml prefix `[mcp_servers.tools.*]` (the policy struct has no `mcp_servers` field — test was masked by a `-- taint` filter). - Extract `LibreFangKernel::snapshot_taint_rules` helper; replace the five inline `self.config.load().taint_rules.clone()` call sites. - Document the snapshot semantics on `KernelConfig.taint_rules`: edits do not propagate to already-connected MCP servers without reload. * feat(runtime-mcp): hot-reload taint_rules + dedicated PATCH /taint endpoint Addresses the two remaining review concerns from #3193: **Hot-reload `[[taint_rules]]`** - `McpServerConfig.taint_rule_sets` is now an `Arc<ArcSwap<Vec<...>>>` shared across every connected MCP server (was: per-server `Vec` snapshot taken at install time). Operators editing `[[taint_rules]]` and reloading config now see the new rules applied to in-flight tool calls without restarting servers. - Kernel owns the master swap; `LibreFangKernel.taint_rules_swap` is initialised at boot and `.store(...)`-updated by `reload_config` (before swapping `self.config` so no scanner observes a window where the two disagree). - Scanner takes a single `.load()` per scan call so config edits during an in-flight argument-tree walk can't mutate rules under the scanner. - `empty_taint_rule_sets_handle()` and `static_taint_rule_sets_handle()` helpers exposed for tests and out-of-kernel callers. **Dedicated PATCH endpoint** - New `PATCH /api/mcp/servers/{name}/taint` accepts only `{ taint_scanning?, taint_policy? }`, removing the dashboard's need to round-trip every other server field through PUT. Closes the silent-drop risk if a future required field gets added without dashboard support. - `useUpdateMcpTaintPolicy` rewritten to call the new endpoint; `TaintPolicyEditor` updated to pass `id` instead of the full `existing` server config. Verified the multi-threaded kernel-test SIGABRT reproduces on `02eb92af` (the original PR head before any fixes), confirming it is pre-existing and not introduced by this change. * fix(kernel): wire `[[taint_rules]]` into reload-plan diff Live integration smoke surfaced a gap in the previous hot-reload commit: appending or editing `[[taint_rules]]` returned `status: no_changes` because `build_reload_plan` did not diff that field, so `should_apply_hot` returned false and the swap-store was never reached. - New `HotAction::ReloadTaintRules` variant; pushed when `old.taint_rules != new.taint_rules`. - Logged from `apply_hot_actions_inner` for audit-trail visibility. - Derive `PartialEq + Eq` on `NamedTaintRuleSet` so the diff compiles (other rule types in the chain — `McpTaintRuleSetAction` and `TaintRuleId` — already had it). - Reverted the temporary `else if` in `reload_config` that performed a partial swap when the plan was empty; the diff now correctly drives the canonical `should_apply_hot` path so no fallback is needed. Live smoke (`PATCH /api/mcp/servers/{name}/taint` + reload-config flows on an isolated `LIBREFANG_HOME`) now reports: - first reload after adding `[[taint_rules]]` → `hot_actions_applied: ["UpdateDefaultModel", "ReloadTaintRules"]` - idempotent second reload → `status: no_changes` - editing only `action = "warn"` → `action = "log"` → `hot_actions_applied: ["ReloadTaintRules"]` * docs(taint): clarify rule_set overlap semantics + fix stale PUT comment Two follow-ups from PR review: 1. The TaintPolicyEditor doc comment said the editor saves through the "existing PUT /api/mcp/servers/{id} endpoint" but actually goes through the new dedicated PATCH /api/mcp/servers/{id}/taint added in this PR. Updated the comment to match and noted why PATCH is preferred (no revalidation of unrelated McpServerConfig fields on every taint edit). 2. When a tool's `rule_sets` list references multiple sets that overlap on the same TaintRuleId, the scanner picks the *most permissive* action (Log > Warn > Block). This is intentional but counter-intuitive — adding an `audit_only` set with `action = log` will silently neutralise any `block` set that overlaps on the same rule. Added a prominent rustdoc paragraph on `McpTaintRuleSetAction` and an inline UI hint next to the dashboard `rule_sets` field (en + zh locales) so operators authoring config aren't surprised. * fix(kernel): reconnect MCP servers on hot-reload when entry config differs `HotAction::ReloadMcpServers` previously only refreshed `effective_mcp_servers` and the health registry; existing connections held their per-server `McpServerConfig` clone (including `taint_policy`, `taint_scanning`, `headers`, `env`, `transport`), so edits via PUT `/api/mcp/servers/{name}`, direct `config.toml` edits, or any non-PATCH reload path silently kept the old policy alive on already-connected servers — a security drift hazard for operators tightening rules and assuming the change took effect. `taint_rule_sets` was already shared via `Arc<ArcSwap>` in the previous fix commit; this change covers the rest of the per-server snapshot. - Snapshot the prior effective list, diff each entry by JSON-serialized content (robust against future field additions, no `PartialEq` required on `McpOAuthConfig` / transport variants / `McpTaintPolicy`). - Servers that are removed or whose entry differs go into `to_reconnect`. - After updating `effective_mcp_servers` and bumping `mcp_generation`, fire-and-forget a task that disconnects the stale slots and calls `connect_mcp_servers()` (idempotent — re-adds servers missing from `mcp_connections` using the freshly-installed effective list). - The PATCH `/api/mcp/servers/{name}/taint` endpoint kept its explicit disconnect+reconnect; this change makes the reload-config path behave the same. * fix(dashboard): disable rule_sets input when tool default = skip The MCP taint scanner short-circuits when a tool's `default = "skip"` and returns before the rule-set walker runs, so any `rule_sets` on the same tool — including audit-only `Log` sets — never fire. The dashboard editor still let operators fill `rule_sets` on a `default = skip` tool, which silently became dead config and could give a false sense of "I have audit logging on this noisy tool". - Disable the `rule_sets` Input when `policy.default === "skip"` so new values can't be added via the UI. - Show a warning hint when the saved policy already has `rule_sets` alongside `default = skip`, telling the operator to switch back to `default = scan` if they want audit-only behaviour. - Existing `rule_sets` values are preserved in state (not silently dropped on save) so toggling default back to `scan` restores them. Adds `mcp.taint_skip_rule_sets_hint` in en/zh locales. * fix(runtime-mcp,api): warn on typo'd rule_set + document JSONPath limits Two follow-up fixes for the granular MCP taint policy review: **Low #3 — JSONPath dotted-key limitation** The pattern parser splits on `.` and treats `[` as the start of array notation, so a JSON object key containing `.` or `[` (e.g. `"a.b"`, `"items[0]"`) cannot be addressed precisely. Quoted-segment syntax (`$.headers."content-type"`) is also not parsed. In practice MCP tool schemas rarely use such keys, but the matcher fails closed (no false positive skip) when it bites. - Add a "Limitation" section to `jsonpath_matches` rustdoc. - New regression test `test_jsonpath_dotted_keys_are_known_limitation` pins the current behaviour so a future change has to opt in. **Low #4 — silent typos in `tool_policy.rule_sets`** `lookup_rule_set_action` previously returned `None` when a referenced name was missing from the registry, so an operator typo (e.g. `"audit_typo"` instead of `"audit"`) became a silent no-op. Now: - New `warn_unknown_rule_set_once(name, tool)` emits a one-shot WARN on first scan that observes a missing name, deduped per process via a static `OnceLock<Mutex<HashSet<String>>>`. - Wired into `lookup_rule_set_action` at the existing skip site. - New `GET /api/mcp/taint-rules` exposes the registered `[[taint_rules]]` names + actions so the dashboard can validate inputs *before* the operator hits Save (separate commit handles the frontend wiring; the endpoint is read-only and reuses the existing auth chain). - Regression test verifies the unknown-name path returns `None` and doesn't panic / corrupt the dedup set on repeated calls. * fix(api): register `list_mcp_taint_rules` with the OpenAPI generator The handler was added to the router in the previous commit but I forgot to list it in `openapi.rs::ApiDoc`, so `openapi.json` was silently re-generated without the new path and the dashboard's typed client wouldn't see the route in the contract. Trivial follow-up — adds one line under the MCP block. * feat(dashboard): inline validation for unknown taint rule_set names Closes the dashboard half of Low #4 from the #3193 review. Backend already emits a one-shot WARN for unknown names; this wires the new `GET /api/mcp/taint-rules` endpoint into the editor so typos surface *before* the operator hits Save. - `listMcpTaintRules` (api.ts) + `useMcpTaintRules` (queries/mcp.ts) wrap the registry endpoint with a 5-minute staleTime — operators don't expect rule-set renames to land instantly and there's no hot churn here. - `mcpKeys.taintRules()` factory follows the existing hierarchy (`mcpKeys.all` invalidation still nukes it). - `TaintPolicyEditor` derives `knownRuleSetNames: Set<string>` once and threads it down to each `ToolPolicyRow`. - Each row computes `unknownRuleSetNames` from the comma-typed input and renders a red inline hint listing the offenders. Falls back silently when the registry is empty (`knownRuleSetNames.size === 0`) so a fresh kernel without `[[taint_rules]]` doesn't false-positive. - New `mcp.taint_rule_sets_unknown_hint` in en/zh locales. - Regenerated `openapi.json` to include the new path (the pre-commit hook missed the previous backend-only commit because the registration in `openapi.rs` was added in the immediately following commit).
5 tasks
This was referenced May 18, 2026
houko
added a commit
that referenced
this pull request
May 19, 2026
* fix(api): preserve operator-hand-edited non-schema env keys on sidecar save upsert_sidecar_block previously rewrote the entire [sidecar_channels.env] table on every save, silently dropping operator hand-edits such as PYTHONPATH=/custom, HTTP_PROXY, locale settings, and even legacy inline TELEGRAM_BOT_TOKEN entries. Make the form's ownership of the env table explicit: callers now pass managed_env_keys — the set of non-secret schema field keys the configure form is the source of truth for. Within env: - A managed key with a non-empty form value overwrites the existing entry. - A managed key absent / empty in the form is removed. - Every other key in the existing env table is preserved untouched. Secret-typed schema fields never appear in config.toml (they live in secrets.env), so they are intentionally excluded from the managed set. Add coverage in sidecar_toml_test: - preserves_non_schema_env_keys_on_replace - removes_schema_managed_env_keys_when_form_clears_them Total in this test file: 4 → 8. * fix(api): use kernel.home_dir() in all channel handlers (codex fix #1 generalize) The original codex fix #1 only converted configure_sidecar_channel. Five sibling handlers in the same file kept resolving via the local librefang_home() helper (LIBREFANG_HOME env + ~/.librefang fallback), bypassing the kernel's authoritative KernelConfig.home_dir in any case where the two diverge: - configure_channel - remove_channel - create_channel_instance - update_channel_instance_handler - delete_channel_instance Replace every site with state.kernel.home_dir().to_path_buf() so they honour the same single source of truth as the rest of the HTTP surface. Delete the now-unused private librefang_home() helper (the unrelated cross-crate helper in routes/system.rs is untouched and still serves agent_templates.rs). The five channel-instance integration tests that previously seeded a config.toml into DiskHomeGuard's standalone tempdir now seed into state.kernel.home_dir() — that's the authoritative path the handlers read. DiskHomeGuard is retained as an RAII guard that keeps LIBREFANG_HOME pointed at a benign tempdir so any code still consulting the env var doesn't escape into the developer's real ~/.librefang. write_discord_instances also pins config_version to types::CONFIG_VERSION so the kernel's load_config migration path doesn't clobber the minimal fixture on first read. * fix(api): kill_on_drop on skills subprocess (codex fix #3 generalize) skills.rs auto-install command spawned via tokio::process::Command inside a tokio::time::timeout(300s) had no kill_on_drop(true), so a timeout (or any dropped Future) would leave the install command running detached in the background. Same defect class as the sidecar --describe subprocess in codex review fix #3, which already added kill_on_drop there. Add .kill_on_drop(true) before .output() so the child is SIGKILLed when the timeout future is dropped. Sweep of all Command::new in crates/librefang-api/src/routes/ confirms this was the only remaining async tokio::process::Command paired with a timeout. The std::process::Command calls in config.rs (ps / tasklist / hostname) and system.rs (hostname) are synchronous, run unguarded with no timeout, and complete in milliseconds — outside the kill_on_drop defect class. * fix(channels,api): accept quoted values in secrets.env parser (consistency with dotenv) librefang_channels::sidecar::parse_secrets_env previously did a naive find('=') with no quote stripping, so an operator hand-editing secrets.env with the common dotenv convention KEY="value" got the literal quote characters propagated into the sidecar child's environment. Tighten the contract: surrounding whitespace is trimmed, and a matched pair of outer ASCII single or double quotes on the value is stripped (mirroring dotenv conventions). Mismatched / one-sided quotes are left verbatim so a value that legitimately contains a single quote isn't silently corrupted. Escape sequences (\n etc.) are explicitly out of scope — documented in the function doc-comment. Add unit coverage in librefang-channels: - parse_secrets_env_strips_double_quotes - parse_secrets_env_strips_single_quotes - parse_secrets_env_keeps_internal_quotes - parse_secrets_env_keeps_mismatched_outer_quotes - parse_secrets_env_trims_whitespace - parse_secrets_env_handles_empty_quoted_value - parse_secrets_env_skips_comments_and_blanks The inline secrets.env reader in routes/channels.rs::configure_sidecar_channel extracts KEYS only (set membership check against schema's secret field names) — quotes never appear inside dotenv KEYS, so no value-side stripping is needed there. Add a leading comment pointing future readers at the channels-crate helper in case the shadow-detection ever grows a value comparison. * test(api): cover include-shadow detection (positive + commented false-positive) configure_sidecar_refuses_when_include_owns_sidecars already covers the positive case (an include with a real [[sidecar_channels]] entry triggers 409). Add configure_sidecar_refuses_even_on_commented_sidecar_string to document the substring-match heuristic's conservative false-positive surface: an included file that only mentions [[sidecar_channels]] inside a TOML comment still triggers 409. The test pins the behaviour so a future refactor that switches to a full TOML parse of included files makes a visible decision rather than silently relaxing the check. Total channels_routes_test: 37 → 38. * style(api): drop redundant deref in upsert_sidecar_block (clippy) `env_table.remove(*key)` triggered clippy::explicit-auto-deref under `-D warnings` (Rust 1.95). `Table::remove` accepts `&str` and Rust autoderefs `&&str` for us — same effect, no clippy fight. Spotted in the workspace clippy pass after fix #6.
8 tasks
houko
added a commit
that referenced
this pull request
May 20, 2026
Replace the 1747-line in-process librefang-channels::discord adapter (Gateway WS v10 + REST v10) with an out-of-process Python sidecar at sdk/python/librefang/sidecar/adapters/discord.py (stdlib-only). Behaviour parity: - GET /gateway/bot URL discovery + WSS connect with ?v=10&encoding=json - HELLO -> IDENTIFY/RESUME -> DISPATCH state machine (READY captures bot_user_id / session_id / resume_gateway_url; INVALID_SESSION resumable preserves state, non-resumable clears; RECONNECT raises) - MESSAGE_CREATE / MESSAGE_UPDATE -> message event with self-skip, ignore_bots, allowed_users / allowed_guilds filters - Attachment-takes-priority-over-slash-command (Image / Video / Voice / File by MIME prefix; audio/file warn-and-drop on companion text) - Discriminator-aware display name; mention detection via mentions[] array + <@id> / <@!id> tags + case-insensitive mention_patterns - POST /channels/{id}/messages with 2000-UTF-16-unit chunking - POST /channels/{id}/typing for typing indicators - account_id injection into message metadata Three improvements over the Rust adapter: 1. **Periodic client-side heartbeats**. The Rust adapter captured heartbeat_interval from HELLO but never sent its own heartbeats — connections silently dropped after ~45s with code=4000 and re-IDENTIFY'd, losing the session every minute. The sidecar runs proper periodic heartbeats with the RFC-mandated random jitter on the first beat, plus a server-initiated heartbeat slides the scheduler's next-deadline so we never double-beat back-to-back. 2. **429 retry-with-Retry-After**. The Rust api_send_message warned on 429 and returned Ok(()) (fail-open silent message loss); the sidecar honours Retry-After and retries once before logging-and- continuing on the second 429. 3. **select-gated frame waits**. The WebSocket reader uses select() to wait for the socket to become readable BEFORE consuming a frame, so mid-frame stalls can't race a heartbeat tick. Known-fatal close codes (4004 / 4013 / 4014) raise rather than reconnect so the supervisor's circuit-breaker stops a hard config error instead of looping. Two regressions to call out (both match the telegram-sidecar #5241 precedent; flagged so operators aren't surprised): - **Live Discord-guild-role RBAC is gone.** ChannelRoleQuery is a Rust trait the sidecar process cannot implement, so role_query is None for Discord post-migration, the kernel's resolve_role_for_sender falls through to the default-deny Viewer branch, and [channel_role_mapping.discord] is no longer consulted. Workaround: enumerate authorised operators under [users] with channel_bindings = { discord = [...] } and an explicit role. - **Per-channel proxy override (#4795) is not wired through.** The sidecar honours HTTP_PROXY / HTTPS_PROXY / ALL_PROXY via stdlib but has no DISCORD_PROXY_URL env var yet. Filed as a follow-up. Cascade removal: - src/discord.rs deleted; lib.rs mod decl, channels-allowlist.txt entry, cargo features in librefang-channels + librefang-api (incl. membership in all-channels / all-channels-no-email / core-channels / mini) removed - DiscordConfig struct + Default impl + canonical deny_unknown_fields rustdoc anchor moved to SlackConfig; ChannelsConfig.discord field + default removed; validation hook removed - channel_bridge.rs: import, spawn block, find_channel_info! arm, check_channel! arm, default-empty test assertion removed - routes/channels.rs: ChannelMeta entry + 5 match arms removed; live-test discord branch (POST to discord.com/api/v10) removed; SIDECAR_CATALOG discord entry added; instance_helper_tests reanchored on slack; downstream_send_failure_returns_502 retired - routes/config.rs ch!(discord) call removed; is_writable_config_path test fixture switched to slack - kernel channel_sender for_each_channel_field! macro entry + EXPECTED name-list entry removed - CLI: librefang setup discord wizard arm + channel list row removed; TUI ChannelDef removed - openclaw migrator: discord channel input now produces a SkippedItem with a sidecar-redirect reason rather than writing [channels.discord] to the output (mirrors how telegram was handled in #5241); test_policy_migration retargeted at slack - openfang test fixtures reanchored on [channels.slack] - librefang.toml.example + cli init_default_config.toml: [channels.discord] block replaced with a commented [[sidecar_channels]] template - docs (en + zh): configuration/page.mdx, configuration/channels/page.mdx, integrations/channels/page.mdx, integrations/channels/core/page.mdx — [channels.discord] blocks replaced with [[sidecar_channels]] templates + a per-channel summary of what the sidecar preserves vs. what changed - ChannelType::Discord enum variant kept (still used by the bridge / router; same pattern as ChannelType::Telegram after #5241) New env-var knobs (read by the sidecar from [sidecar_channels.env]): DISCORD_ALLOWED_GUILDS, DISCORD_ALLOWED_USERS, DISCORD_INTENTS (default 37376), DISCORD_IGNORE_BOTS (default true), DISCORD_MENTION_PATTERNS, DISCORD_ACCOUNT_ID. DISCORD_BOT_TOKEN goes to ~/.librefang/secrets.env. Operator action required: an existing [channels.discord] block is no longer recognised — re-declare as a [[sidecar_channels]] running python3 -m librefang.sidecar.adapters.discord. Verification: - cargo check --workspace --lib: clean - cargo check --workspace --features librefang-api/all-channels: clean - cargo clippy --workspace --all-targets --features librefang-api/all-channels -- -D warnings: zero warnings - cargo test -p librefang-channels --features all-channels --lib: 824/824 - cargo test -p librefang-channels --features all-channels --test bridge_integration_test: 27/27 - cargo test -p librefang-types --lib: 842/842 - cargo test -p librefang-kernel --lib: 1085/1085 - cargo test -p librefang-migrate --lib: 51/51 - cargo test -p librefang-api --lib --features all-channels: 692/692 - cargo test -p librefang-api --test channels_routes_test --features all-channels: 38/38 - cd sdk/python && pytest: 293/293 (69 new for discord including test_handle_payload_dispatch_does_not_signal_heartbeat_sent contract test for #3)
houko
added a commit
that referenced
this pull request
May 20, 2026
…5299) Replace the 1747-line in-process librefang-channels::discord adapter (Gateway WS v10 + REST v10) with an out-of-process Python sidecar at sdk/python/librefang/sidecar/adapters/discord.py (stdlib-only). Behaviour parity: - GET /gateway/bot URL discovery + WSS connect with ?v=10&encoding=json - HELLO -> IDENTIFY/RESUME -> DISPATCH state machine (READY captures bot_user_id / session_id / resume_gateway_url; INVALID_SESSION resumable preserves state, non-resumable clears; RECONNECT raises) - MESSAGE_CREATE / MESSAGE_UPDATE -> message event with self-skip, ignore_bots, allowed_users / allowed_guilds filters - Attachment-takes-priority-over-slash-command (Image / Video / Voice / File by MIME prefix; audio/file warn-and-drop on companion text) - Discriminator-aware display name; mention detection via mentions[] array + <@id> / <@!id> tags + case-insensitive mention_patterns - POST /channels/{id}/messages with 2000-UTF-16-unit chunking - POST /channels/{id}/typing for typing indicators - account_id injection into message metadata Three improvements over the Rust adapter: 1. **Periodic client-side heartbeats**. The Rust adapter captured heartbeat_interval from HELLO but never sent its own heartbeats — connections silently dropped after ~45s with code=4000 and re-IDENTIFY'd, losing the session every minute. The sidecar runs proper periodic heartbeats with the RFC-mandated random jitter on the first beat, plus a server-initiated heartbeat slides the scheduler's next-deadline so we never double-beat back-to-back. 2. **429 retry-with-Retry-After**. The Rust api_send_message warned on 429 and returned Ok(()) (fail-open silent message loss); the sidecar honours Retry-After and retries once before logging-and- continuing on the second 429. 3. **select-gated frame waits**. The WebSocket reader uses select() to wait for the socket to become readable BEFORE consuming a frame, so mid-frame stalls can't race a heartbeat tick. Known-fatal close codes (4004 / 4013 / 4014) raise rather than reconnect so the supervisor's circuit-breaker stops a hard config error instead of looping. Two regressions to call out (both match the telegram-sidecar #5241 precedent; flagged so operators aren't surprised): - **Live Discord-guild-role RBAC is gone.** ChannelRoleQuery is a Rust trait the sidecar process cannot implement, so role_query is None for Discord post-migration, the kernel's resolve_role_for_sender falls through to the default-deny Viewer branch, and [channel_role_mapping.discord] is no longer consulted. Workaround: enumerate authorised operators under [users] with channel_bindings = { discord = [...] } and an explicit role. - **Per-channel proxy override (#4795) is not wired through.** The sidecar honours HTTP_PROXY / HTTPS_PROXY / ALL_PROXY via stdlib but has no DISCORD_PROXY_URL env var yet. Filed as a follow-up. Cascade removal: - src/discord.rs deleted; lib.rs mod decl, channels-allowlist.txt entry, cargo features in librefang-channels + librefang-api (incl. membership in all-channels / all-channels-no-email / core-channels / mini) removed - DiscordConfig struct + Default impl + canonical deny_unknown_fields rustdoc anchor moved to SlackConfig; ChannelsConfig.discord field + default removed; validation hook removed - channel_bridge.rs: import, spawn block, find_channel_info! arm, check_channel! arm, default-empty test assertion removed - routes/channels.rs: ChannelMeta entry + 5 match arms removed; live-test discord branch (POST to discord.com/api/v10) removed; SIDECAR_CATALOG discord entry added; instance_helper_tests reanchored on slack; downstream_send_failure_returns_502 retired - routes/config.rs ch!(discord) call removed; is_writable_config_path test fixture switched to slack - kernel channel_sender for_each_channel_field! macro entry + EXPECTED name-list entry removed - CLI: librefang setup discord wizard arm + channel list row removed; TUI ChannelDef removed - openclaw migrator: discord channel input now produces a SkippedItem with a sidecar-redirect reason rather than writing [channels.discord] to the output (mirrors how telegram was handled in #5241); test_policy_migration retargeted at slack - openfang test fixtures reanchored on [channels.slack] - librefang.toml.example + cli init_default_config.toml: [channels.discord] block replaced with a commented [[sidecar_channels]] template - docs (en + zh): configuration/page.mdx, configuration/channels/page.mdx, integrations/channels/page.mdx, integrations/channels/core/page.mdx — [channels.discord] blocks replaced with [[sidecar_channels]] templates + a per-channel summary of what the sidecar preserves vs. what changed - ChannelType::Discord enum variant kept (still used by the bridge / router; same pattern as ChannelType::Telegram after #5241) New env-var knobs (read by the sidecar from [sidecar_channels.env]): DISCORD_ALLOWED_GUILDS, DISCORD_ALLOWED_USERS, DISCORD_INTENTS (default 37376), DISCORD_IGNORE_BOTS (default true), DISCORD_MENTION_PATTERNS, DISCORD_ACCOUNT_ID. DISCORD_BOT_TOKEN goes to ~/.librefang/secrets.env. Operator action required: an existing [channels.discord] block is no longer recognised — re-declare as a [[sidecar_channels]] running python3 -m librefang.sidecar.adapters.discord. Verification: - cargo check --workspace --lib: clean - cargo check --workspace --features librefang-api/all-channels: clean - cargo clippy --workspace --all-targets --features librefang-api/all-channels -- -D warnings: zero warnings - cargo test -p librefang-channels --features all-channels --lib: 824/824 - cargo test -p librefang-channels --features all-channels --test bridge_integration_test: 27/27 - cargo test -p librefang-types --lib: 842/842 - cargo test -p librefang-kernel --lib: 1085/1085 - cargo test -p librefang-migrate --lib: 51/51 - cargo test -p librefang-api --lib --features all-channels: 692/692 - cargo test -p librefang-api --test channels_routes_test --features all-channels: 38/38 - cd sdk/python && pytest: 293/293 (69 new for discord including test_handle_payload_dispatch_does_not_signal_heartbeat_sent contract test for #3)
houko
pushed a commit
that referenced
this pull request
May 20, 2026
…irect tests PR self-review caught four issues; this commit fixes all of them. Fix #1 + #2: SeenSet docstring corrections ========================================== The old docstring claimed two things that weren't true: * "the adapter classes keep those names available as @Property shims pointing at this class's ``ids`` / ``order`` attributes" — there are no @Property shims; tests were rewritten to read ``adapter._seen.ids`` directly. * Listed bluesky as a SeenSet consumer — bluesky's ``_mark_seen`` is a server-side ``updateSeen`` REST POST, NOT a dedupe shim; the name collision is coincidental. Both claims fixed. Webex's empty-id → False quirk is now also called out explicitly so future readers don't think the shared behaviour is universal. Fix #3: direct unit tests for the shared modules ================================================ Added three new test files: * ``tests/test_common.py`` (40 tests) — covers split_message (passthrough, exact-limit, hard-cut, newline-prefer, empty, unicode), split_csv (empty, whitespace, dropped empties, order), parse_retry_after (missing, integer, decimal, floor, custom floor, max-cap, garbage, negative), SeenSet (fresh, repeat, distinct ids, empty-id, eviction, contains, len, thread safety with 8 concurrent workers, int ids), and http_request (200 JSON, request metadata recording, lowercased response headers, 4xx/5xx surfaced via status not raise, empty body, non-JSON, default timeout, default method). * ``tests/test_ws.py`` (13 tests) — RFC 6455 constants (WS_GUID, opcodes, MAX_FRAME_PAYLOAD), ``_parse_url`` (wss, ws-with-port, default-path, query-string preservation, non-ws reject, missing-host reject), and ``WebSocketClient.__init__`` defaults. Full socket-level coverage (handshake, frame round-trip) needs a real local WS server fixture that's out of scope for this refactor PR. * ``tests/test_sidecar_fakes.py`` (15 tests) — HdrShim, FakeResp CM protocol, FakeUrlopen script handling, call-metadata recording (URL/method/timeout/headers/body), JSON + form-encoded body decoding, 3-tuple response-headers variant, 4xx/5xx HTTPError raising, script-exhaustion AssertionError, empty/bytes body passthrough, back-compat underscore aliases. Fix #4: actually hoist MAX_BACKOFF_SECS / RETRY_AFTER_DEFAULT_SECS ================================================================= The previous round-2 commit added these constants to ``librefang.sidecar.common`` and the commit message claimed every adapter would pick up the canonical values from there. **None did** — all 16 adapters that used them kept their local definitions unchanged, so the hoist was theoretical. This commit actually rewires the imports. Each of the 16 adapters that referenced ``MAX_BACKOFF_SECS = 60.0`` now imports it from ``common``; 11 adapters likewise for ``RETRY_AFTER_DEFAULT_SECS = 30.0``. Local duplicate definitions are removed. A handful of adapters have local overrides that aren't ``60.0`` or ``30.0`` (e.g. webex's stricter ``RETRY_AFTER_DEFAULT_SECS = 30.0`` matches but it has a 30-second cap not 60; mastodon's ``RETRY_AFTER_DEFAULT_SECS = 60.0`` is also canonical but different semantics) — those are intentionally left as local strategy knobs. Verification ============ ``pytest sdk/python/tests/`` — **1095 passed in 5.05s** (was 1027 in 5.00s; the new test files add 68). Diff: 20 files changed, +786 / -105.
This was referenced May 20, 2026
houko
added a commit
that referenced
this pull request
May 21, 2026
Surfaces from the post-#5445 audit (6th in the dingtalk/qq/omnibus review chain — first verdict that wasn't BROKEN/NEEDS_HOTFIX). All three nits are doc/UX cleanups, none are runtime regressions: 1. **Docstring claim #3 was wrong** — said "429 Retry-After honoured on every outbound POST", but only Cloud API uses `_cloud_post_with_retry`; the gateway path calls `_http_request` directly and raises on any non-2xx. Soften the claim to "Cloud-API outbound POSTs only" + explain when gateway-side retry would matter (operators proxying the local Baileys gateway behind a rate-limiting reverse-proxy). 2. **WHATSAPP_VERIFY_TOKEN unset failed silently** — Meta's subscription handshake returned 403 with no log line pointing back to the missing env var; operators saw "subscription failed" in the Meta dashboard with nothing in their daemon logs. Now warns at __init__, matching the WHATSAPP_APP_SECRET pattern already there. 3. **WHATSAPP_GROUP_POLICY is dead config** — Cloud API webhook payloads don't surface a group/conversation distinction (we hardcode `is_group=False` at `_handle_post_webhook`), and gateway mode delegates inbound entirely to the Node Baileys gateway which never calls back into the sidecar's `should_handle_message` filter. So `WHATSAPP_GROUP_POLICY` has zero effect today. Mark the schema field with an explicit "(currently inert)" note in the label so operators don't waste time setting it. Kept the field itself for forward-compat — Meta has been rolling out group-chat support to the Cloud API gradually and we want the schema to be ready when it lands. Drive-by: dropped the misleading send-voice docstring claim (lines 22-23 + 36-39 promised a `{gateway}/message/send-voice` multipart upload route that doesn't exist in code; rationale comment about the daemon's ChannelContent::Voice always carrying a URL at the dispatch boundary explains why we don't need it). Test: - New test_get_verify_empty_self_token_rejects asserts BOTH the __init__ warn fires AND the handshake fails closed. Regression guard for #2 — without this an attacker who guesses the empty hub.verify_token could subscribe their own callback URL. - cd sdk/python && pytest tests/test_whatsapp_adapter.py — 79 passed (was 78; +1 regression guard). This closes the audit chain. WhatsApp itself is structurally clean — natural routing key (phone) is preserved as channel_id both directions, the bug family from #5417 → #5449 doesn't apply.
This was referenced May 22, 2026
2 tasks
This was referenced May 22, 2026
Closed
houko
pushed a commit
that referenced
this pull request
May 28, 2026
…eshold, LLM confidence, KV mirror retirement, instrumented spawn, fuzzy categories, configurable prompt cap, CHANGELOG, comment fix 10 follow-ups raised on the code review of the prior two commits in this PR. All within the same memory-system scope. * #1 root user_id is now a constant sentinel UUID (00000000-0000-0000-0000-72006f0074a0, exported as `ROOT_API_KEY_USER_ID`) rather than `UserId::from_name("root")`. The from_name UUIDv5 lives inside `LIBREFANG_USER_NAMESPACE`, so an operator-registered `[users] name = "root"` would have silently inherited the master credential's ACL + per-user budget cap. The sentinel falls outside that namespace; AuthManager returns None for it and the fail-open Owner-default ACL applies. Regression test `root_api_key_user_id_does_not_collide_with_any_named_user` pins the non-collision invariant against {root, admin, owner, system, operator, user}. * #3 `HotAction::UpdateProactiveMemory` now also calls `substrate.set_consolidation_duplicate_threshold(...)`, so when `POST /api/config/reload` swaps in a new `[proactive_memory] duplicate_threshold`, the periodic global consolidation sweep picks it up alongside the per-agent on-demand consolidate. Without this the per-agent path picked up the new value but the global sweep stayed on the old one — exactly the inconsistency H5 set out to remove. `ConsolidationEngine` switched to an `Arc<AtomicU32>` threshold (f32 bits) so the setter takes `&self`, which is required because the hot-reload code path holds only `Arc<MemorySubstrate>`. `docs/operations/config-reload.md` row updated to call out the new behaviour. * #4 `build_extraction_prompt` now asks the LLM to emit a per-memory `confidence` field with a brief calibration guide; the parser reads it, clamps to [0, 1], and stashes the value in `metadata["confidence"]` and `MemoryItem.confidence` so the C3 insert path actually lands a non-default value in the `confidence` column. Missing field still defaults to 1.0 (matches the rule-based extractor's prior behaviour — never silently drops a memory). Tests `parse_extraction_propagates_confidence_to_metadata` and `parse_extraction_clamps_confidence_to_unit_interval` pin the new behaviour. * #5 The KV `memory:*` mirror is gone — fully retired, not just "non-load-bearing". All `structured.set("memory:*", ...)` / `structured.delete("memory:*", ...)` / `list_kv` scans that walked the mirror have been deleted from `import_memories`, `add_with_decision`'s ADD + UPDATE branches, `add_with_level`, `delete`, `update`, `reset`, `clear_level`, `cleanup_expired_sessions`, the eviction loop, and the consolidation merge-loser path. The read path was already on semantic (C1); leaving the writes in place would have grown the mirror without bound and risked future divergence regressions. `test_delete_memory` rewritten to assert behaviour through `search()` (the trait-level contract) instead of probing the underlying KV store. Any legacy `memory:*` entries from older installs are silently ignored. * #6 The detached auto-consolidate `tokio::spawn` is wrapped in a `tracing::info_span!("auto_consolidate", task = "auto_consolidate", agent = ...)` via `.instrument(span)`, so a panic inside the consolidate future surfaces in tracing output (instead of disappearing silently the way bare-spawn panics do) and operators can grep `task = "auto_consolidate"` to find the detached work. * #7 Category allowlist match is now case-insensitive + tolerant of trailing `s` on either side. `"Preferences"` / `"PREFERENCE"` / `"preferences"` all snap to a configured `"preference"` and the canonical configured spelling lands in the column. Test `parse_extraction_fuzzy_matches_category_case_and_plural` pins the four variants. * #8 `format_context_max_chars` is now a field on `ProactiveMemoryConfig` (default 8000 chars / ~2000 tokens); the store's `format_context_with_query` / `format_context` read it from the live config and pass it to `format_memories_with_budget(memories, max_chars)`. The trait fallback (`DefaultMemoryExtractor::format_context`, `LlmMemoryExtractor::format_context`) keeps the const default for callers without config access. Operators on 200k+ context windows can now raise the cap via `config.toml` without recompiling. * #2 + #9 New `Fixed` entry in `[Unreleased]` documents the breaking audit-shape change (root-api_key requests now stamp a `user_id` where they previously stamped `None`) and the `add()` behaviour change (no raw-transcript fallback for extraction misses). Both were noted inline in commits but absent from the CHANGELOG. The entry also lists the full audit-sweep scope so an operator can size-up the upgrade impact from one place. * #10 `import_memories` comment rewritten to explain the 0.95 threshold without the confusing "stricter than extraction-time dedup" framing. Drive-by: collapsed three more `clippy::manual_option_zip` sites in `kernel/tests.rs` that the workspace-clippy gate would have caught on the next CI run. Auto-restamped `.secrets.baseline` line numbers shifted by the CHANGELOG insert. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api -p librefang-kernel --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 269 passed * cargo test -p librefang-runtime --lib proactive_memory — 60 passed (5 new follow-up regression tests included) * cargo test -p librefang-api --lib --test memory_routes_integration --test agent_kv_authz_integration --test auth_public_allowlist — all green
houko
added a commit
that referenced
this pull request
May 29, 2026
…cay, dedup, prompt budget, async consolidate) (#5839) * fix(memory): split-brain reads, raw-transcript fallback, RBAC on writes, forget() leak, immortal decay 5 CRITICAL findings from a memory-system audit, all in the proactive memory layer: * C1 split-brain: list()/get() read from the KV mirror while search()/auto_retrieve read from the semantic store. The KV write was best-effort (warn-and-continue) so a failure left rows visible to search but invisible to list. retrieve_memory_items now reads from semantic (the authoritative source); KV writes stay as a non-load- bearing compatibility mirror. * C2 raw-transcript fallback: when the extractor returned no signal, add() stored the verbatim concatenated message content as a session-level memory with no category. This was the dominant source of `category=null` rows and duplicate transcripts on the dashboard. The fallback is removed; callers that want raw content use add_with_level explicitly. * C3 confidence hardcoded to 1.0 + immortal decay: remember_with_embedding_and_peer now honors metadata["confidence"] (clamped to 0..=1, defaulting to 1.0) so the LLM extractor's signal reaches the column. decay_confidence reworked: boost divides the rate instead of multiplying the result and clamping to 1.0. The old formula made any memory with >=2 accesses freeze at confidence 1.0 forever; the new formula keeps "popular memories decay slower" but strictly monotonic (boost capped at MAX_BOOST=4.0). * C4 RBAC on write endpoints: memory_add, memory_update, memory_delete, memory_bulk_delete, memory_reset_agent, memory_clear_level, memory_consolidate, memory_cleanup, memory_export_agent, memory_import_agent, memory_decay, memory_store_relations now route through the namespace guard. New ProactiveMemoryStore wrappers cover the previously-unguarded ops (reset, clear_level, export_all, import_memories, decay_confidence). The root api_key is attributed as an Owner- equivalent AuthenticatedApiUser in middleware so operators using only the master credential keep their POST/PUT/DELETE access. * C5 forget() never marked deleted_at: SemanticStore::forget* now stamps deleted_at alongside deleted=1 so the prune_soft_deleted_memories sweep (filter `deleted_at IS NOT NULL`) can actually hard-delete user-/API-initiated deletions. Without the stamp every soft-deleted row leaked its embedding BLOB forever. consolidation.rs's merge-loser delete now stamps it too. Drive-by: removed a clippy::manual_option_zip in kernel/background_lifecycle.rs flagged while clippy-gating the change. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 269 passed (5 pre-existing tests adapted to the new add()-no-fallback semantics; new regression tests for C1 list-from-semantic, C2 no-fallback, C3 monotonic-decay + extractor-confidence-roundtrip, C4 viewer-denied on every write wrapper, C5 forget* stamps deleted_at) * cargo test -p librefang-api --lib --test memory_routes_integration --test agent_kv_authz_integration --test auth_public_allowlist — all green * chore(codegen): auto-regenerate openapi.json + sdk + schema baselines [skip ci] * fix(memory): tighten dedup thresholds, validate LLM extraction, cap prompt budget, detach auto-consolidate, unify consolidation knob Follow-up sweep on the same memory-system audit as the prior commit — picks off the HIGH findings that were in scope for the same crate / file cluster. * H1 duplicate_threshold tightened. The configured default jumps from 0.5 → 0.85 (mem0's recommended near-duplicate cut-off); both metrics — cosine and Jaccard — agree that 0.5 means "topically related", which let opposite-meaning sentences sharing keywords silently merge. `DefaultMemoryExtractor::decide_action`'s hardcoded same-category 0.5 / cross-category 0.6 UPDATE thresholds rise to 0.7 / 0.8; the 0.95 NOOP gate stays. `import_memories`' hardcoded 0.9 dedup floor rises to 0.95 with a docstring explaining why bulk-import is stricter than extraction-time dedup. * H2 LLM-extraction validation. `parse_llm_extraction_response` now enforces a 4-char content floor (drops "ok" / "no" / single-letter junk that was trivially unique and survived dedup), validates the emitted `category` against the configured `extract_categories` allowlist (out-of-allowlist values downgrade to "general" instead of polluting the dashboard's facets), and caps a single extraction call at MAX_MEMORIES_PER_EXTRACTION=20 rows so a runaway model can't churn the eviction loop. * H4 prompt-injection budget. `format_context` now goes through a shared `format_memories_with_budget` helper capped at FORMAT_CONTEXT_MAX_CHARS=8000 (~2000 tokens). Pre-fix the formatter concatenated everything with no ceiling — 10 retrieved memories × 2000-char MAX_MEMORY_CONTENT_LENGTH could push 20 KB into every request. Excess rows are reported via a "[+N additional memories omitted to keep the prompt within budget]" footer so the truncation is observable in the rendered prompt rather than silent. * H6 auto-consolidation no longer blocks the agent. The every-10 trigger in `auto_memorize` now `tokio::spawn`s the consolidate call instead of awaiting it inline; the next agent turn doesn't pay for the O(n²) merge pass plus its SQLite transaction. The detached future borrows nothing from `self` thanks to `ProactiveMemoryStore`'s manual Clone over Arc'd inner state. * H5 single source of truth for the consolidation threshold. `ConsolidationEngine` gains a `duplicate_threshold` field (defaulting to 0.85 to match the new config default) with a `set_duplicate_threshold` setter; `MemorySubstrate` exposes a passthrough; kernel boot pushes `config.proactive_memory. duplicate_threshold` down to the engine. The periodic global consolidation sweep and the on-demand `ProactiveMemoryStore::consolidate` now agree on what counts as a near-duplicate. Keeps the existing 142 callers of `MemorySubstrate::open_in_memory(decay_rate)` source-compatible (no signature break). H7 / H8 from the same audit were already addressed by the C3 fix in the previous commit (popular-memory immortality + dead extraction_threshold). H3 (memory_store/recall vs auto_memorize/ retrieve being disconnected tool surfaces) is intentional product shape rather than a bug — left out. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 269 passed * cargo test -p librefang-runtime --lib (proactive_memory) — 57 passed, including the 5 new regressions (parse_extraction_drops_sub_minimum_content, parse_extraction_downgrades_unknown_category, parse_extraction_preserves_category_when_allowlist_empty, parse_extraction_caps_total_memories_per_call, format_context_caps_prompt_budget_with_truncation_marker). * cargo test -p librefang-api --test memory_routes_integration --test agent_kv_authz_integration --test auth_public_allowlist — all green. * fix(memory): review-followups — sentinel root user_id, hot-reload threshold, LLM confidence, KV mirror retirement, instrumented spawn, fuzzy categories, configurable prompt cap, CHANGELOG, comment fix 10 follow-ups raised on the code review of the prior two commits in this PR. All within the same memory-system scope. * #1 root user_id is now a constant sentinel UUID (00000000-0000-0000-0000-72006f0074a0, exported as `ROOT_API_KEY_USER_ID`) rather than `UserId::from_name("root")`. The from_name UUIDv5 lives inside `LIBREFANG_USER_NAMESPACE`, so an operator-registered `[users] name = "root"` would have silently inherited the master credential's ACL + per-user budget cap. The sentinel falls outside that namespace; AuthManager returns None for it and the fail-open Owner-default ACL applies. Regression test `root_api_key_user_id_does_not_collide_with_any_named_user` pins the non-collision invariant against {root, admin, owner, system, operator, user}. * #3 `HotAction::UpdateProactiveMemory` now also calls `substrate.set_consolidation_duplicate_threshold(...)`, so when `POST /api/config/reload` swaps in a new `[proactive_memory] duplicate_threshold`, the periodic global consolidation sweep picks it up alongside the per-agent on-demand consolidate. Without this the per-agent path picked up the new value but the global sweep stayed on the old one — exactly the inconsistency H5 set out to remove. `ConsolidationEngine` switched to an `Arc<AtomicU32>` threshold (f32 bits) so the setter takes `&self`, which is required because the hot-reload code path holds only `Arc<MemorySubstrate>`. `docs/operations/config-reload.md` row updated to call out the new behaviour. * #4 `build_extraction_prompt` now asks the LLM to emit a per-memory `confidence` field with a brief calibration guide; the parser reads it, clamps to [0, 1], and stashes the value in `metadata["confidence"]` and `MemoryItem.confidence` so the C3 insert path actually lands a non-default value in the `confidence` column. Missing field still defaults to 1.0 (matches the rule-based extractor's prior behaviour — never silently drops a memory). Tests `parse_extraction_propagates_confidence_to_metadata` and `parse_extraction_clamps_confidence_to_unit_interval` pin the new behaviour. * #5 The KV `memory:*` mirror is gone — fully retired, not just "non-load-bearing". All `structured.set("memory:*", ...)` / `structured.delete("memory:*", ...)` / `list_kv` scans that walked the mirror have been deleted from `import_memories`, `add_with_decision`'s ADD + UPDATE branches, `add_with_level`, `delete`, `update`, `reset`, `clear_level`, `cleanup_expired_sessions`, the eviction loop, and the consolidation merge-loser path. The read path was already on semantic (C1); leaving the writes in place would have grown the mirror without bound and risked future divergence regressions. `test_delete_memory` rewritten to assert behaviour through `search()` (the trait-level contract) instead of probing the underlying KV store. Any legacy `memory:*` entries from older installs are silently ignored. * #6 The detached auto-consolidate `tokio::spawn` is wrapped in a `tracing::info_span!("auto_consolidate", task = "auto_consolidate", agent = ...)` via `.instrument(span)`, so a panic inside the consolidate future surfaces in tracing output (instead of disappearing silently the way bare-spawn panics do) and operators can grep `task = "auto_consolidate"` to find the detached work. * #7 Category allowlist match is now case-insensitive + tolerant of trailing `s` on either side. `"Preferences"` / `"PREFERENCE"` / `"preferences"` all snap to a configured `"preference"` and the canonical configured spelling lands in the column. Test `parse_extraction_fuzzy_matches_category_case_and_plural` pins the four variants. * #8 `format_context_max_chars` is now a field on `ProactiveMemoryConfig` (default 8000 chars / ~2000 tokens); the store's `format_context_with_query` / `format_context` read it from the live config and pass it to `format_memories_with_budget(memories, max_chars)`. The trait fallback (`DefaultMemoryExtractor::format_context`, `LlmMemoryExtractor::format_context`) keeps the const default for callers without config access. Operators on 200k+ context windows can now raise the cap via `config.toml` without recompiling. * #2 + #9 New `Fixed` entry in `[Unreleased]` documents the breaking audit-shape change (root-api_key requests now stamp a `user_id` where they previously stamped `None`) and the `add()` behaviour change (no raw-transcript fallback for extraction misses). Both were noted inline in commits but absent from the CHANGELOG. The entry also lists the full audit-sweep scope so an operator can size-up the upgrade impact from one place. * #10 `import_memories` comment rewritten to explain the 0.95 threshold without the confusing "stricter than extraction-time dedup" framing. Drive-by: collapsed three more `clippy::manual_option_zip` sites in `kernel/tests.rs` that the workspace-clippy gate would have caught on the next CI run. Auto-restamped `.secrets.baseline` line numbers shifted by the CHANGELOG insert. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api -p librefang-kernel --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 269 passed * cargo test -p librefang-runtime --lib proactive_memory — 60 passed (5 new follow-up regression tests included) * cargo test -p librefang-api --lib --test memory_routes_integration --test agent_kv_authz_integration --test auth_public_allowlist — all green * fix(api): attribute Owner on no-auth loopback so memory writes aren't 403 The RBAC gating added 12 memory-write ACL checks, but the default `librefang start` (no api_key, loopback bind) takes the no-auth bypass which returned next.run() WITHOUT attaching an AuthenticatedApiUser. Memory write handlers then saw None -> anonymous Viewer fallback -> 403 on every POST/PUT/DELETE /api/memory*, breaking the documented default workflow. No-auth + trusted origin (loopback / LIBREFANG_ALLOW_NO_AUTH) is the same trust level as the root master credential, so attribute the same Owner-equivalent user (ROOT_API_KEY_USER_ID). Non-loopback still fails closed. Add integration tests for both: loopback write != 403, non-loopback no-auth still 401. * fix(memory): read-only recall for listing paths; correct decay doc MEDIUM (#5839): list/get/export/list_all read paths called recall(), which unconditionally bumps access_count + accessed_at. A dashboard polling the memory list would perpetually reset accessed_at = now and inflate access_count — the exact signals the C3 decay logic keys idle/popularity off — so polled listings could keep memories from ever decaying (and turned a GET into a 10k-row write). Add recall_readonly() (shared impl, no bump) and route the four listing/export reads through it; genuine semantic recalls still track access. Regression test asserts recall_readonly leaves access_count untouched while recall() bumps by 1. Also correct the decay_confidence doc: the once-per-hour cadence is enforced by the periodic maintenance scheduler, not an internal throttle (a direct call decays immediately). --------- Co-authored-by: Evan <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
houko
pushed a commit
that referenced
this pull request
May 29, 2026
…loor, partial-status, dedup-strip, test tightening 8 follow-ups from the code review of the prior commit. All in scope for the same proactive-memory layer. * #1 misattributed doc-comment in `proactive.rs` near `AUTO_CONSOLIDATE_EVERY` / `NEGATION_WORDS`. The `/// Negation/contradiction words …` line was orphaned above `AUTO_CONSOLIDATE_EVERY` when the new const got inserted; both consts now carry their own intended docstring. * #2 lowered `STALE_COUNTER_FLOOR` from `AUTO_CONSOLIDATE_EVERY / 2` (5) to `/ 4` (2). The /2 floor cleaned up "stuck at 1..4" agents but also reset slow-burn agents (single auto_memorize per maintenance tick) before they could climb to 10, so a steady 1-call/hour stream effectively never consolidated. /4 keeps the cold-slot eviction directional while letting low-frequency agents still accumulate to the trigger. * #3 `PATCH /api/memory/config` response shape is now explicit about partial success. `body.status` is `"applied"` when the reload succeeded and `"partial"` when the disk write landed but the live reload failed (e.g. operator hand-edited an unrelated section into an invalid shape between PATCH writes). Clients MUST inspect `status`; the HTTP code stays 200 for both branches since the disk write itself succeeded. 207 / 500 were considered and rejected — 500 misrepresents that the request was rejected (it wasn't) and 207 forces every existing client to re-classify success. Mirrors the `import_agent_memory` partial-success pattern. * #4 documented the M13 cost tradeoff. Up to 4 SQLite roundtrips per insertion on the no-embedding fallback path (versus the pre-fix 1), but the embedding-driver path is unaffected and the loop short-circuits as soon as the union hits fetch_limit, so the common case ("first keyword filled the slate") still runs a single query. Operators on the no-embedding path pay the cost on writes only, against an already-unindexed `content LIKE` scan. * #5 defensively strip the M14 `_update_threshold_*` keys from the enriched item right after `decide_action` returns + assert callers don't pre-populate them. Both insertion branches today build their stored metadata from the original `item.metadata` (not `enriched_item.metadata`), so the threshold keys never reach the column — but stripping + asserting is cheap insurance for the next refactorer who repoints either branch at the enriched copy. * #6 relaxed the `extract_search_keywords_returns_multiple_ordered_longest_first` test: no more exact `kws.len() == 4` — the cap and the "longest distinctive word survives" invariants are what we care about, not the exact count. Future additions to `STOP_WORDS` no longer silently break the test. * #7 dropped the unused `_update_threshold_cross_cat` set in `decide_action_honors_config_update_thresholds`. Both candidate memories share the "preference" category, so the cross-cat threshold branch was unreachable. * #8 moved `STALE_COUNTER_FLOOR` from a `fn`-scope `const` to module scope, alongside `AUTO_CONSOLIDATE_EVERY` from which it derives. Matches the repo convention and lets future readers see the floor / trigger relationship at one site. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 273 passed * cargo test -p librefang-api --test memory_routes_integration — 14 passed
houko
pushed a commit
that referenced
this pull request
May 29, 2026
… + M14 regression coverage, comment polish 5 follow-ups from the second review pass on #5850. * B (was #1) M11 done properly. `consolidation_counters` is now `HashMap<String, CounterEntry>` where `CounterEntry` carries both the running count and a `last_touched: DateTime<Utc>` stamp. The maintenance sweep evicts entries whose `last_touched` is older than `STALE_COUNTER_IDLE_WINDOW = 2 hours` (~2× the maintenance rate-limit window), regardless of count. The previous count-threshold fix (followup #2 in the prior commit) mitigated but didn't solve the slow-burn case: any agent firing ≤ 1 × per maintenance window would still be reset before climbing past the count floor. The timestamp-based check closes that gap — an active slot, however slow, is preserved as long as it's been touched within the window; a truly idle slot is reclaimed within ~2 hours of going quiet. * C (was #2) `PATCH /api/memory/config` happy-path test added at `memory_routes_integration::patch_memory_config_hot_reloads_and_reports_applied`. Pre-seeds a minimal `config.toml` (the harness's tempdir previously didn't materialise one, which the file-level docstring flagged as out-of-scope; the docstring updated accordingly) and asserts `body["status"] == "applied"`, `body["reload_error"]` null, and that the PATCHed value round-trips into the response. Without this, the M12 status contract could silently revert and the rest of the suite wouldn't catch it. `RouterHarness._tmp` is exposed as `tmp` to let the test reach the seed location. * D (was #3) M14 strip regression test added at `proactive::tests::add_with_decision_does_not_leak_threshold_keys_to_stored_metadata`. Drives the full `add()` path through `add_with_decision`, then reads back via `list()` and asserts none of the private `_update_threshold_*` / `_embedding` keys leaked into the stored metadata column. Catches the regression where someone repoints the ADD or UPDATE branch at `enriched_item.metadata` (the decision-clone) instead of the original `item.metadata` (the caller's input). * E (was #4) the `debug_assert!` panic messages on the `_update_threshold_*` private keys re-worded from "caller leaked it" to "callers must not pre-populate ..." — neutral phrasing that doesn't presume the caller is buggy. Also added a one-line note that the production path stays safe regardless (the unconditional `insert` overwrites any leaked value before `decide_action` reads it), since `debug_assert!` is compiled out in release. * F (was #5) M13 cost-trade-off comment tightened. The "common case still runs a single query" claim was accurate for agents with sizeable stores (first keyword exhausts fetch_limit) but not for fresh / small stores where no individual keyword has enough matches to short-circuit. The comment now distinguishes the two regimes instead of overgeneralising. Re-stamped `.secrets.baseline` line numbers shifted by the test file edits. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 274 passed (incl. `add_with_decision_does_not_leak_threshold_keys_to_stored_metadata`) * cargo test -p librefang-api --test memory_routes_integration — 15 passed (incl. `patch_memory_config_hot_reloads_and_reports_applied`)
houko
pushed a commit
that referenced
this pull request
May 29, 2026
…epts partial, M14 strips _embedding too, prune rate-limit, chrono::Duration const portability 6 followups from the third review pass on #5850. * #1 M12 test pinned the contract properly. `serde_json::Value` indexed by a missing key returns `Value::Null`, so the prior `assert_eq!(body["reload_error"], Null)` silently passed even if the field had been removed. Now asserts `body.as_object() .contains_key(...)` for `status`, `restart_required`, `reload_error` first, then asserts their values. * #2 strip + test for the `_embedding` private-stash key. `add_with_decision` now also calls `enriched_item.metadata.remove("_embedding")` after `decide_action` returns, so all three private stash keys (`_update_threshold_*` + `_embedding`) get the same defensive treatment. Test renamed to `add_with_decision_does_not_leak_private_stash_keys_to_stored_metadata` and attaches a tiny mock `EmbeddingFn` so the `_embedding` stash path actually fires — the prior assertion against `_embedding` was decorative because the test had no embedding driver configured. * #3 rate-limited the counter prune via a new `last_counter_prune: Arc<Mutex<Option<DateTime<Utc>>>>` and a `maybe_prune_counters` helper that mirrors the `maybe_decay_confidence` / `maybe_cleanup_expired` once-per-hour pattern. Prior to this the prune ran on every `maybe_run_maintenance` call, reachable from `search` / `auto_retrieve` / `consolidate` at potentially many Hz. The retain itself was microseconds, so this is a wash today, but it brings the three maintenance sub-tasks under the same scheduling budget for future scaling. * #4 docstring on `STALE_COUNTER_IDLE_WINDOW_HOURS` re-phrased to describe the slot-keeping guarantee in terms of the maintenance rate-limit window, not "consecutive prune passes" — the old phrasing happened to be true only because the prune wasn't rate-limited yet (now rectified by #3). * #5 M12 test now accepts either `"applied"` or `"partial"` as the body status — both are valid post-fix outcomes; the pre-fix contract had no `status` field at all. The status field's presence (asserted via the #1 fix) is the actual contract we're pinning. Made the test robust to future `KernelConfig::default()` changes that might cause the seeded toml to fail reload validation. * #6 swapped `const STALE_COUNTER_IDLE_WINDOW: chrono::Duration = chrono::Duration::hours(2)` for `const STALE_COUNTER_IDLE_WINDOW_HOURS: i64 = 2` plus `chrono::Duration::hours(STALE_COUNTER_IDLE_WINDOW_HOURS)` at the call site. `chrono::Duration::hours` is a `const fn` at the currently pinned `chrono` minor but the const-ness isn't a stable contract across `0.4.x` versions, so a lockfile bump could silently break the build. The integer-hours + runtime conversion stays valid regardless. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 274 passed (incl. `add_with_decision_does_not_leak_private_stash_keys_to_stored_metadata` with embedding-driver coverage) * cargo test -p librefang-api --test memory_routes_integration — 15 passed (incl. tighter `patch_memory_config_hot_reloads_and_reports_applied` contract assertions)
houko
added a commit
that referenced
this pull request
May 29, 2026
…CH, multi-keyword search, configurable UPDATE thresholds (#5850) * fix(memory): MEDIUM follow-ups — counter map sweep, hot-reload on PATCH, multi-keyword search, configurable UPDATE thresholds Continuation of the audit sweep on the proactive-memory subsystem. 4 MEDIUM findings from the same audit, all in scope for this PR. * M11 `consolidation_counters` HashMap is now actively pruned every maintenance tick. Pre-fix it only swept when the map crossed 1000 entries (truncate-to-500 by count DESC), which delayed cleanup until the leak was observable AND deleted the highest-count entries — exactly the agents about to fire a real consolidate. The new sweep drops every counter that hasn't passed the halfway mark (`< AUTO_CONSOLIDATE_EVERY / 2 = 5`) on each maintenance tick: HashMap::retain in-place, evicts cold entries first, never touches an entry that's about to fire. Added named constant `AUTO_CONSOLIDATE_EVERY = 10` so the trigger + the prune floor stay in lockstep. * M12 `PATCH /api/memory/config` now calls `kernel.reload_config()` after writing `config.toml`, so dashboard saves take effect on the running kernel instead of staying disk-only until restart. Pre-fix the response always reported `restart_required: true`, which confused operators who could see GET return the new values while live behaviour (ProactiveMemoryStore::config, decay engine, etc.) stayed on the boot snapshot. `restart_required` now reflects the actual `ReloadPlan` — false when every diff field hot-reloads, true when any field needs a restart. A reload validation failure is surfaced via the new `reload_error` field instead of swallowing the disk write. * M13 `extract_search_keywords` returns a `Vec<String>` of the top 4 distinctive keywords ordered longest-first (post-stop-word filter, post-dedup), and the caller iterates over them unioning per-keyword LIKE recalls until `fetch_limit` is hit. Pre-fix it collapsed the four candidates to the single longest one, wasting the stop-word filter work on the other three and giving the no-embedding fallback path a frequently-too-generic substring (e.g. "analysis") to match against, OR a too-specific compound term that matched nothing. The fallback to the raw-content LIKE is preserved for the no-distinctive-words case so a near-verbatim duplicate is still detectable. * M14 the `decide_action` UPDATE thresholds are now configurable via two new `ProactiveMemoryConfig` fields: `update_threshold_same_category` (default 0.7) and `update_threshold_cross_category` (default 0.8). The trait method signature stays stable — `add_with_decision` stashes the live config values in the new memory's metadata under `_update_threshold_same_cat` / `_update_threshold_cross_cat`, the default `decide_action` reads them out (falling back to the const defaults for direct trait-method callers), and the LLM-backed extractor inherits the same behaviour via its fallback to the default heuristic on driver failure. This separates the per-insertion conflict-resolution threshold (UPDATE vs ADD) from the post-hoc consolidation threshold (`duplicate_threshold`) — pre-fix both were conflated. Drive-by fmt: two stale formatting hunks `cargo fmt` flagged in `runtime/tool_runner/wasm_skill.rs:159` and `api/tests/memory_routes_integration.rs:518` (neither mine, both inherited from main). Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api -p librefang-kernel --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 273 passed (4 new regression tests: `extract_search_keywords_returns_multiple_ordered_longest_first`, `extract_search_keywords_empty_for_all_stop_words`, `decide_action_honors_config_update_thresholds`, and the existing suite re-verified against the new `update_threshold_*_category` config fields) * cargo test -p librefang-runtime --lib proactive_memory — 60 passed * cargo test -p librefang-api --lib --test memory_routes_integration --test agent_kv_authz_integration --test auth_public_allowlist — all green * fix(memory): review-followups on #5850 — doc-comment fix, threshold floor, partial-status, dedup-strip, test tightening 8 follow-ups from the code review of the prior commit. All in scope for the same proactive-memory layer. * #1 misattributed doc-comment in `proactive.rs` near `AUTO_CONSOLIDATE_EVERY` / `NEGATION_WORDS`. The `/// Negation/contradiction words …` line was orphaned above `AUTO_CONSOLIDATE_EVERY` when the new const got inserted; both consts now carry their own intended docstring. * #2 lowered `STALE_COUNTER_FLOOR` from `AUTO_CONSOLIDATE_EVERY / 2` (5) to `/ 4` (2). The /2 floor cleaned up "stuck at 1..4" agents but also reset slow-burn agents (single auto_memorize per maintenance tick) before they could climb to 10, so a steady 1-call/hour stream effectively never consolidated. /4 keeps the cold-slot eviction directional while letting low-frequency agents still accumulate to the trigger. * #3 `PATCH /api/memory/config` response shape is now explicit about partial success. `body.status` is `"applied"` when the reload succeeded and `"partial"` when the disk write landed but the live reload failed (e.g. operator hand-edited an unrelated section into an invalid shape between PATCH writes). Clients MUST inspect `status`; the HTTP code stays 200 for both branches since the disk write itself succeeded. 207 / 500 were considered and rejected — 500 misrepresents that the request was rejected (it wasn't) and 207 forces every existing client to re-classify success. Mirrors the `import_agent_memory` partial-success pattern. * #4 documented the M13 cost tradeoff. Up to 4 SQLite roundtrips per insertion on the no-embedding fallback path (versus the pre-fix 1), but the embedding-driver path is unaffected and the loop short-circuits as soon as the union hits fetch_limit, so the common case ("first keyword filled the slate") still runs a single query. Operators on the no-embedding path pay the cost on writes only, against an already-unindexed `content LIKE` scan. * #5 defensively strip the M14 `_update_threshold_*` keys from the enriched item right after `decide_action` returns + assert callers don't pre-populate them. Both insertion branches today build their stored metadata from the original `item.metadata` (not `enriched_item.metadata`), so the threshold keys never reach the column — but stripping + asserting is cheap insurance for the next refactorer who repoints either branch at the enriched copy. * #6 relaxed the `extract_search_keywords_returns_multiple_ordered_longest_first` test: no more exact `kws.len() == 4` — the cap and the "longest distinctive word survives" invariants are what we care about, not the exact count. Future additions to `STOP_WORDS` no longer silently break the test. * #7 dropped the unused `_update_threshold_cross_cat` set in `decide_action_honors_config_update_thresholds`. Both candidate memories share the "preference" category, so the cross-cat threshold branch was unreachable. * #8 moved `STALE_COUNTER_FLOOR` from a `fn`-scope `const` to module scope, alongside `AUTO_CONSOLIDATE_EVERY` from which it derives. Matches the repo convention and lets future readers see the floor / trigger relationship at one site. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 273 passed * cargo test -p librefang-api --test memory_routes_integration — 14 passed * fix(memory): review-followups (round 2) — proper M11 idle-window, M12 + M14 regression coverage, comment polish 5 follow-ups from the second review pass on #5850. * B (was #1) M11 done properly. `consolidation_counters` is now `HashMap<String, CounterEntry>` where `CounterEntry` carries both the running count and a `last_touched: DateTime<Utc>` stamp. The maintenance sweep evicts entries whose `last_touched` is older than `STALE_COUNTER_IDLE_WINDOW = 2 hours` (~2× the maintenance rate-limit window), regardless of count. The previous count-threshold fix (followup #2 in the prior commit) mitigated but didn't solve the slow-burn case: any agent firing ≤ 1 × per maintenance window would still be reset before climbing past the count floor. The timestamp-based check closes that gap — an active slot, however slow, is preserved as long as it's been touched within the window; a truly idle slot is reclaimed within ~2 hours of going quiet. * C (was #2) `PATCH /api/memory/config` happy-path test added at `memory_routes_integration::patch_memory_config_hot_reloads_and_reports_applied`. Pre-seeds a minimal `config.toml` (the harness's tempdir previously didn't materialise one, which the file-level docstring flagged as out-of-scope; the docstring updated accordingly) and asserts `body["status"] == "applied"`, `body["reload_error"]` null, and that the PATCHed value round-trips into the response. Without this, the M12 status contract could silently revert and the rest of the suite wouldn't catch it. `RouterHarness._tmp` is exposed as `tmp` to let the test reach the seed location. * D (was #3) M14 strip regression test added at `proactive::tests::add_with_decision_does_not_leak_threshold_keys_to_stored_metadata`. Drives the full `add()` path through `add_with_decision`, then reads back via `list()` and asserts none of the private `_update_threshold_*` / `_embedding` keys leaked into the stored metadata column. Catches the regression where someone repoints the ADD or UPDATE branch at `enriched_item.metadata` (the decision-clone) instead of the original `item.metadata` (the caller's input). * E (was #4) the `debug_assert!` panic messages on the `_update_threshold_*` private keys re-worded from "caller leaked it" to "callers must not pre-populate ..." — neutral phrasing that doesn't presume the caller is buggy. Also added a one-line note that the production path stays safe regardless (the unconditional `insert` overwrites any leaked value before `decide_action` reads it), since `debug_assert!` is compiled out in release. * F (was #5) M13 cost-trade-off comment tightened. The "common case still runs a single query" claim was accurate for agents with sizeable stores (first keyword exhausts fetch_limit) but not for fresh / small stores where no individual keyword has enough matches to short-circuit. The comment now distinguishes the two regimes instead of overgeneralising. Re-stamped `.secrets.baseline` line numbers shifted by the test file edits. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 274 passed (incl. `add_with_decision_does_not_leak_threshold_keys_to_stored_metadata`) * cargo test -p librefang-api --test memory_routes_integration — 15 passed (incl. `patch_memory_config_hot_reloads_and_reports_applied`) * fix(memory): review-followups (round 3) — M12 test key-presence + accepts partial, M14 strips _embedding too, prune rate-limit, chrono::Duration const portability 6 followups from the third review pass on #5850. * #1 M12 test pinned the contract properly. `serde_json::Value` indexed by a missing key returns `Value::Null`, so the prior `assert_eq!(body["reload_error"], Null)` silently passed even if the field had been removed. Now asserts `body.as_object() .contains_key(...)` for `status`, `restart_required`, `reload_error` first, then asserts their values. * #2 strip + test for the `_embedding` private-stash key. `add_with_decision` now also calls `enriched_item.metadata.remove("_embedding")` after `decide_action` returns, so all three private stash keys (`_update_threshold_*` + `_embedding`) get the same defensive treatment. Test renamed to `add_with_decision_does_not_leak_private_stash_keys_to_stored_metadata` and attaches a tiny mock `EmbeddingFn` so the `_embedding` stash path actually fires — the prior assertion against `_embedding` was decorative because the test had no embedding driver configured. * #3 rate-limited the counter prune via a new `last_counter_prune: Arc<Mutex<Option<DateTime<Utc>>>>` and a `maybe_prune_counters` helper that mirrors the `maybe_decay_confidence` / `maybe_cleanup_expired` once-per-hour pattern. Prior to this the prune ran on every `maybe_run_maintenance` call, reachable from `search` / `auto_retrieve` / `consolidate` at potentially many Hz. The retain itself was microseconds, so this is a wash today, but it brings the three maintenance sub-tasks under the same scheduling budget for future scaling. * #4 docstring on `STALE_COUNTER_IDLE_WINDOW_HOURS` re-phrased to describe the slot-keeping guarantee in terms of the maintenance rate-limit window, not "consecutive prune passes" — the old phrasing happened to be true only because the prune wasn't rate-limited yet (now rectified by #3). * #5 M12 test now accepts either `"applied"` or `"partial"` as the body status — both are valid post-fix outcomes; the pre-fix contract had no `status` field at all. The status field's presence (asserted via the #1 fix) is the actual contract we're pinning. Made the test robust to future `KernelConfig::default()` changes that might cause the seeded toml to fail reload validation. * #6 swapped `const STALE_COUNTER_IDLE_WINDOW: chrono::Duration = chrono::Duration::hours(2)` for `const STALE_COUNTER_IDLE_WINDOW_HOURS: i64 = 2` plus `chrono::Duration::hours(STALE_COUNTER_IDLE_WINDOW_HOURS)` at the call site. `chrono::Duration::hours` is a `const fn` at the currently pinned `chrono` minor but the const-ness isn't a stable contract across `0.4.x` versions, so a lockfile bump could silently break the build. The integer-hours + runtime conversion stays valid regardless. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 274 passed (incl. `add_with_decision_does_not_leak_private_stash_keys_to_stored_metadata` with embedding-driver coverage) * cargo test -p librefang-api --test memory_routes_integration — 15 passed (incl. tighter `patch_memory_config_hot_reloads_and_reports_applied` contract assertions) * fix(memory): review-followups (round 4) — docstring honesty + direct strip helper + non-empty partial-error 3 follow-ups from the fourth review pass on #5850. All three are about closing gaps between what the code does and what the docstrings / tests claim it does. * A `STALE_COUNTER_IDLE_WINDOW_HOURS` docstring: the "reclaimed within ~2 hours of going quiet" claim was accurate before the round-3 prune rate-limit landed, after which the worst-case reclaim latency is 2-3 hours (idle window + up to one prune rate-limit period because the prune itself only runs ≤ once per hour). Re-phrased with explicit upper/lower bounds; deleted the duplicate "previous fix (round-1 followup #2)" paragraph that had ended up in the doc twice during the round-2 edit. * B `strip_private_stash_keys` extracted into a module-level function driven by a single `ADD_WITH_DECISION_PRIVATE_STASH_KEYS` const, and unit-tested directly via `strip_private_stash_keys_removes_all_private_keys`. The integration test `add_with_decision_does_not_leak_private_stash_keys_to_stored_metadata` only catches a *coordinated two-step regression* (strip removed AND ADD/UPDATE branch repointed at `enriched_item.metadata`) — the current ADD path bypasses `enriched_item.metadata` entirely, so single-step regressions of either kind pass that test. The prior commit's docstring claimed "the strip code on the post-decide path is the only thing keeping it out of the stored column", which was wrong — `item.metadata` (the caller's input) is what actually keeps the keys out today. Updated the docstring to match reality and added the direct unit test on the helper to cover single-step strip regressions. * C `reload_error` partial-branch assertion in `patch_memory_config_hot_reloads_and_reports_applied` now also rejects empty / whitespace-only strings. `is_string()` alone passed `""` / `" "` / any other zero-info value — operators would see status=partial with a useless error blob and have no actionable diagnostic. Trimmed-non-empty makes the contract honest about what "carries the validator output" means. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 275 passed (1 new: `strip_private_stash_keys_removes_all_private_keys`) * cargo test -p librefang-api --test memory_routes_integration — 15 passed --------- Co-authored-by: Evan <[email protected]>
This was referenced Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bumps docker/login-action from 3 to 4.
Release notes
Sourced from docker/login-action's releases.
... (truncated)
Commits
b45d80fMerge pull request #929 from crazy-max/node24176cb9cnode 24 as default runtimecad8984Merge pull request #920 from docker/dependabot/npm_and_yarn/aws-sdk-dependenc...92cbcb2chore: update generated content5a2d6a7build(deps): bump the aws-sdk-dependencies group with 2 updates44512b6Merge pull request #928 from docker/dependabot/npm_and_yarn/docker/actions-to...28737a5chore: update generated contentdac0793build(deps): bump@docker/actions-toolkitfrom 0.76.0 to 0.77.062029f3Merge pull request #919 from docker/dependabot/npm_and_yarn/actions/core-3.0.008c8f06chore: update generated contentDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)