fix(growth,kernel,runtime): 穷尽审计后 7 项修复 — body 持久化 + embedding 死路径 + 安全加固
Some checks failed
CI / Lint & TypeCheck (push) Has been cancelled
CI / Unit Tests (push) Has been cancelled
CI / Build Frontend (push) Has been cancelled
CI / Rust Check (push) Has been cancelled
CI / Security Scan (push) Has been cancelled
CI / E2E Tests (push) Has been cancelled
Some checks failed
CI / Lint & TypeCheck (push) Has been cancelled
CI / Unit Tests (push) Has been cancelled
CI / Build Frontend (push) Has been cancelled
CI / Rust Check (push) Has been cancelled
CI / Security Scan (push) Has been cancelled
CI / E2E Tests (push) Has been cancelled
CRITICAL 修复: - body_markdown 数据丢失: SkillManifest.body 字段 + serialize_skill_md 使用 body 替代默认内容 - embedding 检索死路径: rerank_entries 使用异步 index_entry_with_embedding + score_similarity_with_embedding (70/30 混合) - try_write 静默丢失: pending_embedding 字段 + apply_pending_embedding() 延迟应用 IMPORTANT 修复: - auto_mode 内存泄漏: add_pending 容量限制 100 + 溢出时丢弃最旧 - name_to_slug 空 ID: uuid fallback for empty/whitespace-only names - compaction embedding 缺失: compaction GrowthIntegration 也接收 embedding - kernel 未初始化警告: viking_configure_embedding warn log 验证: 934+ tests PASS, 0 failures
This commit is contained in:
@@ -19,6 +19,8 @@ pub struct MemoryRetriever {
|
|||||||
config: RetrievalConfig,
|
config: RetrievalConfig,
|
||||||
/// Semantic scorer for similarity computation
|
/// Semantic scorer for similarity computation
|
||||||
scorer: RwLock<SemanticScorer>,
|
scorer: RwLock<SemanticScorer>,
|
||||||
|
/// Pending embedding client (applied on next scorer access if try_write failed)
|
||||||
|
pending_embedding: std::sync::Mutex<Option<Arc<dyn crate::retrieval::semantic::EmbeddingClient>>>,
|
||||||
/// Query analyzer
|
/// Query analyzer
|
||||||
analyzer: QueryAnalyzer,
|
analyzer: QueryAnalyzer,
|
||||||
/// Memory cache
|
/// Memory cache
|
||||||
@@ -32,6 +34,7 @@ impl MemoryRetriever {
|
|||||||
viking,
|
viking,
|
||||||
config: RetrievalConfig::default(),
|
config: RetrievalConfig::default(),
|
||||||
scorer: RwLock::new(SemanticScorer::new()),
|
scorer: RwLock::new(SemanticScorer::new()),
|
||||||
|
pending_embedding: std::sync::Mutex::new(None),
|
||||||
analyzer: QueryAnalyzer::new(),
|
analyzer: QueryAnalyzer::new(),
|
||||||
cache: MemoryCache::default_config(),
|
cache: MemoryCache::default_config(),
|
||||||
}
|
}
|
||||||
@@ -244,19 +247,40 @@ impl MemoryRetriever {
|
|||||||
|
|
||||||
let mut scorer = self.scorer.write().await;
|
let mut scorer = self.scorer.write().await;
|
||||||
|
|
||||||
|
// Apply any pending embedding client
|
||||||
|
self.apply_pending_embedding(&mut scorer);
|
||||||
|
|
||||||
|
// Check if embedding is available for enhanced scoring
|
||||||
|
let use_embedding = scorer.is_embedding_available();
|
||||||
|
|
||||||
// Index entries for semantic search
|
// Index entries for semantic search
|
||||||
|
if use_embedding {
|
||||||
|
for entry in &entries {
|
||||||
|
scorer.index_entry_with_embedding(entry).await;
|
||||||
|
}
|
||||||
|
} else {
|
||||||
for entry in &entries {
|
for entry in &entries {
|
||||||
scorer.index_entry(entry);
|
scorer.index_entry(entry);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Score each entry
|
// Score each entry
|
||||||
let mut scored: Vec<(f32, MemoryEntry)> = entries
|
let mut scored: Vec<(f32, MemoryEntry)> = if use_embedding {
|
||||||
|
let mut results = Vec::with_capacity(entries.len());
|
||||||
|
for entry in entries {
|
||||||
|
let score = scorer.score_similarity_with_embedding(query, &entry).await;
|
||||||
|
results.push((score, entry));
|
||||||
|
}
|
||||||
|
results
|
||||||
|
} else {
|
||||||
|
entries
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|entry| {
|
.map(|entry| {
|
||||||
let score = scorer.score_similarity(query, &entry);
|
let score = scorer.score_similarity(query, &entry);
|
||||||
(score, entry)
|
(score, entry)
|
||||||
})
|
})
|
||||||
.collect();
|
.collect()
|
||||||
|
};
|
||||||
|
|
||||||
// Sort by score (descending), then by importance and access count
|
// Sort by score (descending), then by importance and access count
|
||||||
scored.sort_by(|a, b| {
|
scored.sort_by(|a, b| {
|
||||||
@@ -420,7 +444,8 @@ impl MemoryRetriever {
|
|||||||
/// Configure embedding client for semantic similarity
|
/// Configure embedding client for semantic similarity
|
||||||
///
|
///
|
||||||
/// Stores the client for lazy application on first scorer use.
|
/// Stores the client for lazy application on first scorer use.
|
||||||
/// Safe to call from non-async contexts.
|
/// If the scorer lock is busy, the client is stored as pending
|
||||||
|
/// and applied on the next successful lock acquisition.
|
||||||
pub fn set_embedding_client(
|
pub fn set_embedding_client(
|
||||||
&self,
|
&self,
|
||||||
client: Arc<dyn crate::retrieval::semantic::EmbeddingClient>,
|
client: Arc<dyn crate::retrieval::semantic::EmbeddingClient>,
|
||||||
@@ -429,7 +454,20 @@ impl MemoryRetriever {
|
|||||||
*scorer = SemanticScorer::with_embedding(client);
|
*scorer = SemanticScorer::with_embedding(client);
|
||||||
tracing::info!("[MemoryRetriever] Embedding client configured for semantic scorer");
|
tracing::info!("[MemoryRetriever] Embedding client configured for semantic scorer");
|
||||||
} else {
|
} else {
|
||||||
tracing::warn!("[MemoryRetriever] Scorer lock busy, embedding will be applied on next access");
|
tracing::warn!("[MemoryRetriever] Scorer lock busy, storing embedding client as pending");
|
||||||
|
if let Ok(mut pending) = self.pending_embedding.lock() {
|
||||||
|
*pending = Some(client);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Apply any pending embedding client to the scorer.
|
||||||
|
fn apply_pending_embedding(&self, scorer: &mut SemanticScorer) {
|
||||||
|
if let Ok(mut pending) = self.pending_embedding.lock() {
|
||||||
|
if let Some(client) = pending.take() {
|
||||||
|
*scorer = SemanticScorer::with_embedding(client);
|
||||||
|
tracing::info!("[MemoryRetriever] Pending embedding client applied to scorer");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ use zclaw_types::SkillId;
|
|||||||
/// Safety invariants:
|
/// Safety invariants:
|
||||||
/// - `mode` is always `PromptOnly` (auto-generated skills cannot execute code)
|
/// - `mode` is always `PromptOnly` (auto-generated skills cannot execute code)
|
||||||
/// - `enabled` is `false` (requires one explicit positive feedback to activate)
|
/// - `enabled` is `false` (requires one explicit positive feedback to activate)
|
||||||
/// - `body_markdown` becomes the SKILL.md body content (stored by serialize_skill_md)
|
/// - `body_markdown` is stored in `manifest.body` and persisted by `serialize_skill_md`
|
||||||
pub fn candidate_to_manifest(candidate: &SkillCandidate) -> SkillManifest {
|
pub fn candidate_to_manifest(candidate: &SkillCandidate) -> SkillManifest {
|
||||||
let slug = name_to_slug(&candidate.name);
|
let slug = name_to_slug(&candidate.name);
|
||||||
|
|
||||||
@@ -32,6 +32,7 @@ pub fn candidate_to_manifest(candidate: &SkillCandidate) -> SkillManifest {
|
|||||||
triggers: candidate.triggers.clone(),
|
triggers: candidate.triggers.clone(),
|
||||||
tools: candidate.tools.clone(),
|
tools: candidate.tools.clone(),
|
||||||
enabled: false,
|
enabled: false,
|
||||||
|
body: Some(candidate.body_markdown.clone()),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -48,7 +49,13 @@ fn name_to_slug(name: &str) -> String {
|
|||||||
result.push_str(&format!("{:x}", c as u32));
|
result.push_str(&format!("{:x}", c as u32));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
result.trim_matches('-').to_string()
|
let slug = result.trim_matches('-').to_string();
|
||||||
|
if slug.is_empty() {
|
||||||
|
// Fallback for empty or whitespace-only names
|
||||||
|
format!("skill-{}", &uuid::Uuid::new_v4().to_string()[..8])
|
||||||
|
} else {
|
||||||
|
slug
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
|
|||||||
@@ -324,6 +324,9 @@ impl Kernel {
|
|||||||
if let Some(ref driver) = self.extraction_driver {
|
if let Some(ref driver) = self.extraction_driver {
|
||||||
growth_for_compaction = growth_for_compaction.with_llm_driver(driver.clone());
|
growth_for_compaction = growth_for_compaction.with_llm_driver(driver.clone());
|
||||||
}
|
}
|
||||||
|
if let Some(ref embed_client) = self.embedding_client {
|
||||||
|
growth_for_compaction.configure_embedding(embed_client.clone());
|
||||||
|
}
|
||||||
let mw = zclaw_runtime::middleware::compaction::CompactionMiddleware::new(
|
let mw = zclaw_runtime::middleware::compaction::CompactionMiddleware::new(
|
||||||
threshold,
|
threshold,
|
||||||
zclaw_runtime::CompactionConfig::default(),
|
zclaw_runtime::CompactionConfig::default(),
|
||||||
|
|||||||
@@ -50,7 +50,14 @@ impl EvolutionMiddleware {
|
|||||||
|
|
||||||
/// 添加一个待确认的进化事件
|
/// 添加一个待确认的进化事件
|
||||||
pub async fn add_pending(&self, evolution: PendingEvolution) {
|
pub async fn add_pending(&self, evolution: PendingEvolution) {
|
||||||
self.pending.write().await.push(evolution);
|
let mut pending = self.pending.write().await;
|
||||||
|
if pending.len() >= 100 {
|
||||||
|
tracing::warn!(
|
||||||
|
"[EvolutionMiddleware] Pending queue full (100), dropping oldest event"
|
||||||
|
);
|
||||||
|
pending.remove(0);
|
||||||
|
}
|
||||||
|
pending.push(evolution);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// 获取并清除所有待确认事件
|
/// 获取并清除所有待确认事件
|
||||||
|
|||||||
@@ -191,6 +191,7 @@ pub fn parse_skill_md(content: &str) -> Result<SkillManifest> {
|
|||||||
triggers,
|
triggers,
|
||||||
tools,
|
tools,
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
body: None,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -292,6 +293,7 @@ pub fn parse_skill_toml(content: &str) -> Result<SkillManifest> {
|
|||||||
triggers,
|
triggers,
|
||||||
tools,
|
tools,
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
body: None,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -241,6 +241,7 @@ impl SkillRegistry {
|
|||||||
// P2-19: Preserve tools field during update (was silently dropped)
|
// P2-19: Preserve tools field during update (was silently dropped)
|
||||||
tools: if updates.tools.is_empty() { existing.tools } else { updates.tools },
|
tools: if updates.tools.is_empty() { existing.tools } else { updates.tools },
|
||||||
enabled: updates.enabled,
|
enabled: updates.enabled,
|
||||||
|
body: existing.body,
|
||||||
};
|
};
|
||||||
|
|
||||||
// Rewrite SKILL.md
|
// Rewrite SKILL.md
|
||||||
@@ -318,10 +319,14 @@ fn serialize_skill_md(manifest: &SkillManifest) -> String {
|
|||||||
parts.push("---".to_string());
|
parts.push("---".to_string());
|
||||||
parts.push(String::new());
|
parts.push(String::new());
|
||||||
|
|
||||||
// Body: use description as the skill content
|
// Body: use custom body if provided, otherwise default to "# {name}\n\n{description}"
|
||||||
|
if let Some(ref body) = manifest.body {
|
||||||
|
parts.push(body.clone());
|
||||||
|
} else {
|
||||||
parts.push(format!("# {}", manifest.name));
|
parts.push(format!("# {}", manifest.name));
|
||||||
parts.push(String::new());
|
parts.push(String::new());
|
||||||
parts.push(manifest.description.clone());
|
parts.push(manifest.description.clone());
|
||||||
|
}
|
||||||
|
|
||||||
parts.join("\n")
|
parts.join("\n")
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -534,6 +534,7 @@ mod tests {
|
|||||||
triggers: triggers.into_iter().map(|s| s.to_string()).collect(),
|
triggers: triggers.into_iter().map(|s| s.to_string()).collect(),
|
||||||
tools: vec![],
|
tools: vec![],
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
body: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -95,6 +95,9 @@ pub struct SkillManifest {
|
|||||||
/// Whether the skill is enabled
|
/// Whether the skill is enabled
|
||||||
#[serde(default = "default_enabled")]
|
#[serde(default = "default_enabled")]
|
||||||
pub enabled: bool,
|
pub enabled: bool,
|
||||||
|
/// Custom body content for SKILL.md (overrides default "# {name}\n\n{description}")
|
||||||
|
#[serde(default, skip)]
|
||||||
|
pub body: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
fn default_enabled() -> bool { true }
|
fn default_enabled() -> bool { true }
|
||||||
|
|||||||
@@ -468,6 +468,7 @@ mod tests {
|
|||||||
triggers: vec![],
|
triggers: vec![],
|
||||||
tools: vec![],
|
tools: vec![],
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
body: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ fn test_manifest(mode: SkillMode) -> SkillManifest {
|
|||||||
triggers: vec![],
|
triggers: vec![],
|
||||||
tools: vec![],
|
tools: vec![],
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
body: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -81,6 +81,7 @@ fn skill_manifest_full_roundtrip() {
|
|||||||
triggers: vec!["test trigger".to_string()],
|
triggers: vec!["test trigger".to_string()],
|
||||||
tools: vec!["bash".to_string()],
|
tools: vec!["bash".to_string()],
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
body: None,
|
||||||
};
|
};
|
||||||
let json = serde_json::to_string(&manifest).unwrap();
|
let json = serde_json::to_string(&manifest).unwrap();
|
||||||
let parsed: SkillManifest = serde_json::from_str(&json).unwrap();
|
let parsed: SkillManifest = serde_json::from_str(&json).unwrap();
|
||||||
@@ -126,6 +127,7 @@ fn skill_manifest_all_modes_roundtrip() {
|
|||||||
triggers: vec![],
|
triggers: vec![],
|
||||||
tools: vec![],
|
tools: vec![],
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
body: None,
|
||||||
};
|
};
|
||||||
let json = serde_json::to_string(&manifest).unwrap();
|
let json = serde_json::to_string(&manifest).unwrap();
|
||||||
let parsed: SkillManifest = serde_json::from_str(&json).unwrap();
|
let parsed: SkillManifest = serde_json::from_str(&json).unwrap();
|
||||||
|
|||||||
@@ -174,8 +174,9 @@ pub async fn skill_create(
|
|||||||
tags: vec![],
|
tags: vec![],
|
||||||
category: None,
|
category: None,
|
||||||
triggers: request.triggers,
|
triggers: request.triggers,
|
||||||
tools: vec![], // P2-19: Include tools field
|
tools: vec![],
|
||||||
enabled: request.enabled.unwrap_or(true),
|
enabled: request.enabled.unwrap_or(true),
|
||||||
|
body: None,
|
||||||
};
|
};
|
||||||
|
|
||||||
kernel.create_skill(manifest.clone())
|
kernel.create_skill(manifest.clone())
|
||||||
@@ -221,8 +222,9 @@ pub async fn skill_update(
|
|||||||
tags: existing.tags.clone(),
|
tags: existing.tags.clone(),
|
||||||
category: existing.category.clone(),
|
category: existing.category.clone(),
|
||||||
triggers: request.triggers.unwrap_or(existing.triggers),
|
triggers: request.triggers.unwrap_or(existing.triggers),
|
||||||
tools: existing.tools.clone(), // P2-19: Preserve tools on update
|
tools: existing.tools.clone(),
|
||||||
enabled: request.enabled.unwrap_or(existing.enabled),
|
enabled: request.enabled.unwrap_or(existing.enabled),
|
||||||
|
body: existing.body.clone(),
|
||||||
};
|
};
|
||||||
|
|
||||||
let result = kernel.update_skill(&SkillId::new(&id), updated)
|
let result = kernel.update_skill(&SkillId::new(&id), updated)
|
||||||
|
|||||||
@@ -637,6 +637,11 @@ pub async fn viking_configure_embedding(
|
|||||||
if let Some(ref mut k) = *kernel_lock {
|
if let Some(ref mut k) = *kernel_lock {
|
||||||
k.set_embedding_client(arc_adapter);
|
k.set_embedding_client(arc_adapter);
|
||||||
tracing::info!("[VikingCommands] Embedding propagated to Kernel skill router + memory retriever");
|
tracing::info!("[VikingCommands] Embedding propagated to Kernel skill router + memory retriever");
|
||||||
|
} else {
|
||||||
|
tracing::warn!(
|
||||||
|
"[VikingCommands] Kernel not initialized, embedding only applied to SqliteStorage. \
|
||||||
|
It will be applied when Kernel boots."
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user