fix: harden slop ledger rescue paths
This commit is contained in:
@@ -721,7 +721,7 @@ pub fn slop(_app: &mut App, arg: Option<&str>) -> CommandResult {
|
||||
let _ = writeln!(
|
||||
out,
|
||||
"[{}] {} ({:?} | {:?}) — {}",
|
||||
&entry.id[..8],
|
||||
crate::slop_ledger::short_id(&entry.id),
|
||||
entry.bucket.as_str(),
|
||||
entry.severity,
|
||||
entry.status,
|
||||
|
||||
@@ -545,7 +545,7 @@ pub const COMMANDS: &[CommandInfo] = &[
|
||||
name: "slop",
|
||||
aliases: &["canzha"],
|
||||
usage: "/slop [query|export]",
|
||||
description_id: MessageId::CmdHelpDescription,
|
||||
description_id: MessageId::CmdSlopDescription,
|
||||
},
|
||||
];
|
||||
|
||||
|
||||
@@ -12,7 +12,7 @@ use std::collections::hash_map::DefaultHasher;
|
||||
use std::hash::{Hash, Hasher};
|
||||
use std::path::PathBuf;
|
||||
use std::sync::{Arc, Mutex as StdMutex};
|
||||
use std::time::{Duration, Instant};
|
||||
use std::time::{Duration, Instant, SystemTime};
|
||||
|
||||
use anyhow::Result;
|
||||
use futures_util::StreamExt;
|
||||
@@ -331,11 +331,10 @@ pub struct Engine {
|
||||
/// Diagnostics collected during the current step's tool calls. Drained
|
||||
/// and forwarded as a synthetic user message before the next API call.
|
||||
pending_lsp_blocks: Vec<crate::lsp::DiagnosticBlock>,
|
||||
/// Cached SlopLedger gate block so `refresh_system_prompt` doesn't hit
|
||||
/// the filesystem on every turn (#2127). `None` = not yet loaded;
|
||||
/// `Some(None)` = loaded, no open entries; `Some(Some(...))` = loaded,
|
||||
/// gate block ready.
|
||||
slop_ledger_gate_cache: Option<Option<String>>,
|
||||
/// Cached SlopLedger gate block keyed by the ledger file's modified time.
|
||||
/// This keeps prompt refreshes cheap while still noticing append/update
|
||||
/// writes from slop ledger tools during the same session.
|
||||
slop_ledger_gate_cache: Option<(Option<SystemTime>, Option<String>)>,
|
||||
}
|
||||
|
||||
// === Internal tool helpers ===
|
||||
@@ -1851,25 +1850,8 @@ impl Engine {
|
||||
|
||||
// SlopLedger completion-gate: inject unresolved slop entries into the
|
||||
// system prompt so the agent can autonomously review them before
|
||||
// claiming the task is done (#2127). Cached to avoid filesystem I/O on
|
||||
// every turn — only re-loaded when the cache is empty (first call or
|
||||
// after invalidation).
|
||||
let gate_block = match &self.slop_ledger_gate_cache {
|
||||
Some(cached) => cached.clone(),
|
||||
None => {
|
||||
let loaded = crate::slop_ledger::SlopLedger::load()
|
||||
.ok()
|
||||
.and_then(|ledger| {
|
||||
if ledger.has_open_entries() {
|
||||
ledger.completion_gate_summary()
|
||||
} else {
|
||||
None
|
||||
}
|
||||
});
|
||||
self.slop_ledger_gate_cache = Some(loaded.clone());
|
||||
loaded
|
||||
}
|
||||
};
|
||||
// claiming the task is done (#2127).
|
||||
let gate_block = self.slop_ledger_gate_block();
|
||||
if let Some(ref block) = gate_block {
|
||||
if let Some(SystemPrompt::Text(prompt_text)) = &mut stable_prompt {
|
||||
prompt_text.push_str("\n\n");
|
||||
@@ -1888,6 +1870,31 @@ impl Engine {
|
||||
}
|
||||
}
|
||||
|
||||
fn slop_ledger_gate_block(&mut self) -> Option<String> {
|
||||
let modified = crate::slop_ledger::SlopLedger::default_path()
|
||||
.ok()
|
||||
.and_then(|path| std::fs::metadata(path).ok())
|
||||
.and_then(|metadata| metadata.modified().ok());
|
||||
|
||||
if let Some((cached_modified, cached_block)) = &self.slop_ledger_gate_cache
|
||||
&& *cached_modified == modified
|
||||
{
|
||||
return cached_block.clone();
|
||||
}
|
||||
|
||||
let loaded = crate::slop_ledger::SlopLedger::load()
|
||||
.ok()
|
||||
.and_then(|ledger| {
|
||||
if ledger.has_open_entries() {
|
||||
ledger.completion_gate_summary()
|
||||
} else {
|
||||
None
|
||||
}
|
||||
});
|
||||
self.slop_ledger_gate_cache = Some((modified, loaded.clone()));
|
||||
loaded
|
||||
}
|
||||
|
||||
fn merge_compaction_summary(&mut self, summary_prompt: Option<SystemPrompt>) {
|
||||
if summary_prompt.is_none() {
|
||||
return;
|
||||
|
||||
@@ -298,6 +298,7 @@ pub enum MessageId {
|
||||
CmdSettingsDescription,
|
||||
CmdSkillDescription,
|
||||
CmdSkillsDescription,
|
||||
CmdSlopDescription,
|
||||
CmdStashDescription,
|
||||
CmdStatusDescription,
|
||||
CmdStatuslineDescription,
|
||||
@@ -531,6 +532,7 @@ pub const ALL_MESSAGE_IDS: &[MessageId] = &[
|
||||
MessageId::CmdSettingsDescription,
|
||||
MessageId::CmdSkillDescription,
|
||||
MessageId::CmdSkillsDescription,
|
||||
MessageId::CmdSlopDescription,
|
||||
MessageId::CmdStashDescription,
|
||||
MessageId::CmdStatusDescription,
|
||||
MessageId::CmdStatuslineDescription,
|
||||
@@ -979,6 +981,7 @@ fn english(id: MessageId) -> &'static str {
|
||||
MessageId::CmdSkillsDescription => {
|
||||
"List local skills (filter by `/skills <prefix>`; --remote browses the curated registry)"
|
||||
}
|
||||
MessageId::CmdSlopDescription => "Inspect or export the SlopLedger",
|
||||
MessageId::CmdStashDescription => {
|
||||
"Park or restore a composer draft (Ctrl+S to push, /stash list/pop)"
|
||||
}
|
||||
@@ -1367,6 +1370,7 @@ fn japanese(id: MessageId) -> Option<&'static str> {
|
||||
MessageId::CmdSkillsDescription => {
|
||||
"ローカルスキルを一覧表示(`/skills <prefix>` で絞り込み、--remote で精選レジストリを参照)"
|
||||
}
|
||||
MessageId::CmdSlopDescription => "Inspect or export the SlopLedger",
|
||||
MessageId::CmdStashDescription => {
|
||||
"コンポーザーの下書きを退避/復元(Ctrl+S で退避、/stash list|pop)"
|
||||
}
|
||||
@@ -1708,6 +1712,7 @@ fn chinese_simplified(id: MessageId) -> Option<&'static str> {
|
||||
MessageId::CmdSkillsDescription => {
|
||||
"列出本地技能(用 `/skills <prefix>` 按名称前缀过滤,--remote 浏览精选注册表)"
|
||||
}
|
||||
MessageId::CmdSlopDescription => "Inspect or export the SlopLedger",
|
||||
MessageId::CmdStashDescription => "暂存或恢复输入草稿(Ctrl+S 暂存,/stash list|pop)",
|
||||
MessageId::CmdStatusDescription => "显示当前运行状态",
|
||||
MessageId::CmdStatuslineDescription => "配置底栏要显示哪些条目",
|
||||
@@ -2045,6 +2050,7 @@ fn portuguese_brazil(id: MessageId) -> Option<&'static str> {
|
||||
MessageId::CmdSkillsDescription => {
|
||||
"Listar skills locais (filtre com `/skills <prefixo>`; --remote navega pelo registro curado)"
|
||||
}
|
||||
MessageId::CmdSlopDescription => "Inspect or export the SlopLedger",
|
||||
MessageId::CmdStashDescription => {
|
||||
"Estacionar ou restaurar rascunho do compositor (Ctrl+S estaciona, /stash list|pop)"
|
||||
}
|
||||
@@ -2436,6 +2442,7 @@ fn spanish_latin_america(id: MessageId) -> Option<&'static str> {
|
||||
MessageId::CmdSkillsDescription => {
|
||||
"Listar skills locales (filtra con `/skills <prefijo>`; --remote navega el registro curado)"
|
||||
}
|
||||
MessageId::CmdSlopDescription => "Inspect or export the SlopLedger",
|
||||
MessageId::CmdStashDescription => {
|
||||
"Estacionar o restaurar borrador del compositor (Ctrl+S estaciona, /stash list|pop)"
|
||||
}
|
||||
|
||||
@@ -304,9 +304,9 @@ impl SlopLedger {
|
||||
}
|
||||
|
||||
/// Append one or more entries. Returns the new entry count and
|
||||
/// the short ids of the appended entries (first 8 chars).
|
||||
/// the short ids of the appended entries.
|
||||
pub fn append(&mut self, entries: Vec<SlopEntry>) -> (usize, Vec<String>) {
|
||||
let ids: Vec<String> = entries.iter().map(|e| e.id[..8].to_string()).collect();
|
||||
let ids: Vec<String> = entries.iter().map(|e| short_id(&e.id)).collect();
|
||||
self.entries.extend(entries);
|
||||
(self.entries.len(), ids)
|
||||
}
|
||||
@@ -375,18 +375,21 @@ impl SlopLedger {
|
||||
status: SlopEntryStatus,
|
||||
cleanup_recommendation: Option<String>,
|
||||
) -> io::Result<Option<&SlopEntry>> {
|
||||
let entry = match self.find_mut(id) {
|
||||
Some(e) => e,
|
||||
None => return Ok(None),
|
||||
let full_id = {
|
||||
let entry = match self.find_mut(id) {
|
||||
Some(e) => e,
|
||||
None => return Ok(None),
|
||||
};
|
||||
entry.status = status;
|
||||
entry.updated_at = chrono::Utc::now().to_rfc3339();
|
||||
if let Some(rec) = cleanup_recommendation {
|
||||
entry.cleanup_recommendation = Some(rec);
|
||||
}
|
||||
entry.id.clone()
|
||||
};
|
||||
entry.status = status;
|
||||
entry.updated_at = chrono::Utc::now().to_rfc3339();
|
||||
if let Some(rec) = cleanup_recommendation {
|
||||
entry.cleanup_recommendation = Some(rec);
|
||||
}
|
||||
self.save()?;
|
||||
// Return a shared ref to the updated entry
|
||||
Ok(self.entries.iter().find(|e| e.id == id))
|
||||
// Return a shared ref to the updated entry.
|
||||
Ok(self.entries.iter().find(|e| e.id == full_id))
|
||||
}
|
||||
|
||||
/// Export all entries as a Markdown string suitable for handoff or
|
||||
@@ -430,7 +433,7 @@ impl SlopLedger {
|
||||
let title = truncate_str(&e.title, 60);
|
||||
out.push_str(&format!(
|
||||
"| {} | {:?} | {:?} | {:?} | {title} | {source} |\n",
|
||||
&e.id[..8],
|
||||
short_id(&e.id),
|
||||
e.severity,
|
||||
e.confidence,
|
||||
e.status
|
||||
@@ -440,7 +443,7 @@ impl SlopLedger {
|
||||
|
||||
// Detailed entries
|
||||
for e in bucket_entries {
|
||||
out.push_str(&format!("### {} — {}\n\n", &e.id[..8], e.title));
|
||||
out.push_str(&format!("### {} — {}\n\n", short_id(&e.id), e.title));
|
||||
out.push_str(&format!("- **Severity**: {:?}\n", e.severity));
|
||||
out.push_str(&format!("- **Confidence**: {:?}\n", e.confidence));
|
||||
out.push_str(&format!("- **Status**: {:?}\n", e.status));
|
||||
@@ -729,7 +732,7 @@ impl ToolSpec for SlopLedgerQueryTool {
|
||||
for entry in &results {
|
||||
out.push_str(&format!(
|
||||
"- [{}] **{}** ({:?} | {:?} | {:?}) — {}\n",
|
||||
&entry.id[..8],
|
||||
short_id(&entry.id),
|
||||
entry.bucket.as_str(),
|
||||
entry.severity,
|
||||
entry.confidence,
|
||||
@@ -806,7 +809,7 @@ impl ToolSpec for SlopLedgerUpdateTool {
|
||||
match ledger.update_status(id, status, cleanup) {
|
||||
Ok(Some(entry)) => Ok(ToolResult::success(format!(
|
||||
"Updated slop ledger entry {} ({}) → {:?}",
|
||||
&entry.id[..8],
|
||||
short_id(&entry.id),
|
||||
entry.title,
|
||||
entry.status
|
||||
))),
|
||||
@@ -912,6 +915,14 @@ fn truncate_str(s: &str, max_chars: usize) -> String {
|
||||
format!("{truncated}…")
|
||||
}
|
||||
|
||||
/// Return a display-safe short id without assuming byte offsets are char
|
||||
/// boundaries. Ledger ids are normally UUIDs, but imported or hand-edited
|
||||
/// ledgers may contain shorter or non-ASCII ids.
|
||||
#[must_use]
|
||||
pub fn short_id(id: &str) -> String {
|
||||
id.chars().take(8).collect()
|
||||
}
|
||||
|
||||
/// Redact sensitive patterns from exported text: API keys and secrets
|
||||
/// paths. Scan the output for known key prefixes (`sk-`, `Bearer `, `dsk-`)
|
||||
/// and replace the token until a whitespace / punctuation boundary with
|
||||
@@ -961,7 +972,7 @@ fn redact_exported_text(text: &mut String) {
|
||||
|
||||
impl SlopLedger {
|
||||
/// Completion-gate / verifier hook: returns `true` when there are
|
||||
/// unresolved slop entries (status `Open` or `Investigate`) that the
|
||||
/// unresolved slop entries (status `Open` or `InProgress`) that the
|
||||
/// agent should review before claiming the task is done.
|
||||
///
|
||||
/// Tools and engine hooks can call this on claim-of-done to surface
|
||||
@@ -1005,7 +1016,7 @@ impl SlopLedger {
|
||||
out.push_str(&format!(
|
||||
"- **{}** `{}` ({:?}/{:?}): {}\n",
|
||||
e.bucket.as_str(),
|
||||
&e.id[..8],
|
||||
short_id(&e.id),
|
||||
e.severity,
|
||||
e.confidence,
|
||||
truncate_str(&e.title, 80),
|
||||
@@ -1062,6 +1073,44 @@ mod tests {
|
||||
assert_eq!(loaded.entries[0].title, "README is outdated");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn short_id_handles_short_and_non_ascii_ids() {
|
||||
assert_eq!(short_id("abc"), "abc");
|
||||
assert_eq!(short_id("abcdefghi"), "abcdefgh");
|
||||
assert_eq!(short_id("残渣-ledger-entry"), "残渣-ledge");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn display_paths_do_not_panic_on_short_or_non_ascii_ids() {
|
||||
let (_tmp, mut ledger) = temp_ledger();
|
||||
|
||||
let mut short = SlopEntry::new(
|
||||
SlopBucket::StaleDocs,
|
||||
SlopSeverity::Low,
|
||||
SlopConfidence::High,
|
||||
"short id".into(),
|
||||
"desc".into(),
|
||||
);
|
||||
short.id = "abc".into();
|
||||
|
||||
let mut unicode = SlopEntry::new(
|
||||
SlopBucket::ToolGaps,
|
||||
SlopSeverity::Medium,
|
||||
SlopConfidence::Medium,
|
||||
"unicode id".into(),
|
||||
"desc".into(),
|
||||
);
|
||||
unicode.id = "残渣-ledger-entry".into();
|
||||
|
||||
let (_total, ids) = ledger.append(vec![short, unicode]);
|
||||
assert_eq!(ids, vec!["abc", "残渣-ledge"]);
|
||||
|
||||
let md = ledger.export_markdown(None, None);
|
||||
assert!(md.contains("| abc |"));
|
||||
assert!(md.contains("| 残渣-ledge |"));
|
||||
assert!(ledger.completion_gate_summary().is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn query_by_bucket() {
|
||||
let (_tmp, mut ledger) = temp_ledger();
|
||||
@@ -1144,6 +1193,29 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn update_status_returns_entry_for_prefix_match() {
|
||||
let (_tmp, mut ledger) = temp_ledger();
|
||||
|
||||
let entry = SlopEntry::new(
|
||||
SlopBucket::NamingDrift,
|
||||
SlopSeverity::Low,
|
||||
SlopConfidence::High,
|
||||
"naming issue".into(),
|
||||
"desc".into(),
|
||||
);
|
||||
let id = entry.id.clone();
|
||||
let prefix = short_id(&id);
|
||||
let _ = ledger.append(vec![entry]);
|
||||
ledger.save().unwrap();
|
||||
|
||||
let result = ledger
|
||||
.update_status(&prefix, SlopEntryStatus::Resolved, None)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(result.map(|entry| entry.id.as_str()), Some(id.as_str()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn export_markdown() {
|
||||
let (_tmp, mut ledger) = temp_ledger();
|
||||
|
||||
Reference in New Issue
Block a user