Skip to content

Commit 366be3e

Browse files
committed
fix(cache): make skill list ordering deterministic to stop KV cache busts
SkillRegistry stores skills in a HashMap, and list() returned self.skills.values() in per-instance randomized order. The 'Available Skills' system-prompt section is built from this, and current_skills_snapshot can hand back a different HashMap instance via its lock-contended self.skills.clone() fallback. Under tool-loop lock contention this emitted the skills section in different orders across calls: a system prompt with identical length but different bytes, which silently invalidated Anthropic's strict-prefix KV cache mid-conversation. This matches the observed KV_CACHE_USAGE signature: system_changed=true with system_chars==baseline_system_chars (same width, different content). Sort list() by name so ordering is stable regardless of HashMap seed or instance. Add a regression test.
1 parent db373f5 commit 366be3e

1 file changed

Lines changed: 49 additions & 2 deletions

File tree

crates/jcode-base/src/skill.rs

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,9 +321,19 @@ impl SkillRegistry {
321321
self.skills.get(name)
322322
}
323323

324-
/// List all available skills
324+
/// List all available skills.
325+
///
326+
/// Sorted by skill name so the ordering is deterministic. The backing store
327+
/// is a `HashMap`, whose iteration order is randomized per instance; without
328+
/// this sort, two snapshots of the same skill set (e.g. the lock-contended
329+
/// `self.skills.clone()` fallback in `current_skills_snapshot`) could emit
330+
/// the "Available Skills" prompt section in different orders. That produces a
331+
/// system prompt with identical length but different bytes, silently busting
332+
/// the Anthropic strict-prefix KV cache mid-conversation.
325333
pub fn list(&self) -> Vec<&Skill> {
326-
self.skills.values().collect()
334+
let mut skills: Vec<&Skill> = self.skills.values().collect();
335+
skills.sort_by(|a, b| a.name.cmp(&b.name));
336+
skills
327337
}
328338

329339
/// Reload a specific skill by name
@@ -705,6 +715,43 @@ mod tests {
705715
.expect("write skill");
706716
}
707717

718+
#[test]
719+
fn list_is_sorted_by_name_regardless_of_insertion_order() {
720+
// The "Available Skills" system-prompt section is built from `list()`.
721+
// The backing store is a HashMap (per-instance randomized iteration
722+
// order), and `current_skills_snapshot` can hand back a *different*
723+
// HashMap instance via its lock-contended `self.skills.clone()` fallback.
724+
// If `list()` did not sort, two snapshots of the same skill set could
725+
// serialize the section in different orders: a same-length but
726+
// different-bytes system prompt that silently busts the KV cache.
727+
let names = ["zebra", "alpha", "mango", "beta", "yak"];
728+
729+
let mut reg_a = SkillRegistry::default();
730+
for name in names {
731+
reg_a
732+
.skills
733+
.insert(name.to_string(), test_skill(name, "d", "c"));
734+
}
735+
736+
// Build a second registry with the reverse insertion order to maximize
737+
// the chance of a differing HashMap layout.
738+
let mut reg_b = SkillRegistry::default();
739+
for name in names.iter().rev() {
740+
reg_b
741+
.skills
742+
.insert(name.to_string(), test_skill(name, "d", "c"));
743+
}
744+
745+
let order_a: Vec<&str> = reg_a.list().iter().map(|s| s.name.as_str()).collect();
746+
let order_b: Vec<&str> = reg_b.list().iter().map(|s| s.name.as_str()).collect();
747+
748+
assert_eq!(order_a, vec!["alpha", "beta", "mango", "yak", "zebra"]);
749+
assert_eq!(
750+
order_a, order_b,
751+
"list() ordering must be identical across HashMap instances"
752+
);
753+
}
754+
708755
#[test]
709756
fn skill_as_memory_entry_formats_invocation_and_prompt() {
710757
let skill = test_skill(

0 commit comments

Comments
 (0)