From 1b6cd57dca6f697350be28f036858146a708e91d Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Wed, 4 Nov 2026 15:50:39 +0800 Subject: [PATCH] =?UTF-8?q?fix(skillmgr):=20=E5=86=85=E6=A0=B8=E6=97=A7?= =?UTF-8?q?=E7=89=88=20skill=20=E6=B0=B8=E4=B8=8D=E6=9B=B4=E6=96=B0=20?= =?UTF-8?q?=E2=80=94=E2=80=94=20=E6=97=A0=20stamp=20=E4=B8=8D=E7=AD=89?= =?UTF-8?q?=E4=BA=8E"=E7=94=A8=E6=88=B7=E6=94=B9=E8=BF=87"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 缺陷（实测，非推测） 给 kernel-plugins 补上 `skill_info(name, section=...)` 的用法说明后， 改了嵌入内容，但**生产上永远不会更新**： /home/hologramagent/skills/kernel-plugins/ mtime 2026-10-26，无 .builtin-sha256 嵌入版本 1.1.0 vs 磁盘版本 1.0.0 `materializeBuiltinSkills` 的判断链是： builtinSkillUnmodified(dir) → 读 .builtin-sha256 没有 stamp ⇒ false ⇒ continue（"用户改过或无法确认，保留人的版本"） 而 stamp 机制是 387ae94（今天早些时候）引入的。生产机上这份 skill 是**更早的内核**写的 —— 它没有 stamp，于是被永久判为"来历不明"。 后果不只是这一份文档：任何在 stamp 机制之前部署的机器，其内核自带 skill 都停在旧版。而 kernel-* 系列正是教模型怎么用工具/记忆/终端的 文档 —— 内核改进了文档，模型永远看不到。 ## 修法：用 frontmatter version 区分两种"没有 stamp" 原判据把两件事混成一件： 用户改过措辞 ⇒ 必须保留 内核在 stamp 之前写的 ⇒ 应该升级 新增 builtinSkillOutdated(dir, embedded)：只在**没有 stamp** 时被调用， 判据是磁盘 frontmatter 的 `version` 严格低于内嵌版本。 安全性分析（写在代码注释里）： 人改措辞 通常不动 version ⇒ 不低于 ⇒ 保留 ✓ 人把 version 改高 ⇒ 保留 ✓ 内核旧版（无 stamp） ⇒ 升级 ✓（这正是要修的） 解析不出版本号 ⇒ 返回 false ⇒ 保留（保守） 误升级的唯一条件是"人把 version 改低"，而那本身就意味着放弃内核版本。 原有的四条保守判据（含 NoStampIsConservative）全部保持绿。 ## 判据抓到了我自己写的 bug TestCompareVersion 立刻红了： compareVersion("1.0","1.0.0")=-1，期望 0 原因：缺失段留成空串 ⇒ Atoi("") 失败 ⇒ 落到字符串比较 ⇒ `"" < "0"` ⇒ 版本号少写一段的 skill 会被无谓地重复升级。改为缺失段视为 "0"。 ## 变异验证 builtinSkillOutdated 恒 return false（= 修复前行为） ⇒ TestMaterializeBuiltinSkills_UpgradesStaleKernelVersion 红： "★ 没有 stamp 的内核旧版（version 0.0.1）没被升级" compareVersion 缺失段改回空串 ⇒ TestCompareVersion 红："compareVersion(\"1.0\",\"1.0.0\")=-1，期望 0" ## 顺带：kernel-plugins v1.1.0 补一节「Reading a skill without burning context」，说明： - 长文档返回目录 + 开头 30 行，目录每行带 (N 行) - `section` 可按编号或标题子串取，大小写不敏感，容忍 `##` 前缀 - 顶层章节返回整棵子树 - 未命中会列出全部可用章节 - 章节边界尊重围栏代码块 ## 验证 go build ./... PASS go vet ./... PASS gofmt -l 干净 go test ./... -count=1 62 包全过，0 FAIL 干净检出（worktree） build + skillmgr 全绿 （包数 63 → 62 是因 clawhubadapter 已在 a0077a2 迁出为外部插件，与本改动无关） --- internal/plugins/skillmgr/builtin.go | 126 +++++++++++++++++- .../skillmgr/builtin/kernel-plugins/SKILL.md | 22 ++- .../plugins/skillmgr/builtin_sync_test.go | 97 ++++++++++++++ 3 files changed, 243 insertions(+), 2 deletions(-) diff --git a/internal/plugins/skillmgr/builtin.go b/internal/plugins/skillmgr/builtin.go index 14791ef7..393cd5d7 100644 --- a/internal/plugins/skillmgr/builtin.go +++ b/internal/plugins/skillmgr/builtin.go @@ -35,6 +35,7 @@ import ( "log" "os" "path/filepath" + "strconv" "strings" ) @@ -97,7 +98,25 @@ func materializeBuiltinSkills(skillsDir string) int { // 已存在：判断是不是"内核写的且没人改过"，是才覆盖。 if _, statErr := os.Stat(dstDir); statErr == nil { if !builtinSkillUnmodified(dstDir) { - continue // 用户改过（或无法确认）：保留人的版本 + // 没有 stamp / 内容被改过 ⇒ 默认保留（人可能改过措辞）。 + // + // ★ 但有一种情况必须升级：这个是**内核在 stamp 机制之前写的** + // 旧版本。判据 = 磁盘上的 frontmatter version 明确低于内嵌版本。 + // + // 为什么需要这一条（实测缺陷）：stamp 机制由 387ae94 引入， + // 而生产机上 kernel-plugins 是更早的内核写的（mtime 2026-10-26）， + // 没有 .builtin-sha256 ⇒ builtinSkillUnmodified 恒为 false + // ⇒ **这份 skill 永不更新**。后果是内核改进了 skill 文档， + // 已部署的机器永远拿不到 —— 而这个 skill 正是教模型怎么用工具的。 + // + // 为什么用 version 而不是内容差异：用户手工改措辞时通常不动 + // version，所以"version 更高"是"内核发布了新版"的可靠信号； + // 反过来，人把 version 改高的情况几乎不存在。 + if !builtinSkillOutdated(dstDir, data) { + continue // 用户改过（或无法确认）：保留人的版本 + } + log.Printf("[skillmgr] upgrading stale builtin skill %s "+ + "(no stamp, on-disk version older than embedded)", name) } if cur, err := os.ReadFile(filepath.Join(dstDir, skillFileName)); err == nil && string(cur) == string(data) { continue // 内容已是最新，不用重写（保留 mtime） @@ -159,3 +178,108 @@ func sha256Hex(b []byte) string { sum := sha256.Sum256(b) return hex.EncodeToString(sum[:]) } + +// builtinSkillOutdated 判断磁盘上的 skill 是否是**内核写的旧版本**。 +// +// 只在没有 stamp 时才被调用（有 stamp 的走 unmodified 判定）。 +// +// 判据：磁盘文件的 frontmatter `version` 严格低于内嵌版本。 +// +// ## 为什么这条判据是安全的 +// +// 它只在"两份都能解析出版本号"且"磁盘更低"时返回 true： +// +// 人手工改措辞 → 通常不动 version ⇒ 不低于 ⇒ 保留（正确） +// 人手工把版本改高 → 不触发升级 ⇒ 保留（正确，虽然几乎不会发生） +// 内核旧版（无 stamp）→ 磁盘版本更低 ⇒ 升级（这正是要修的缺陷） +// 解析不出版本号 → 返回 false ⇒ 保留（保守） +// +// 也就是说：**误升级的唯一条件是"人把 version 改低了"**，而那种改动 +// 本身就是要放弃内核版本的意思，却反而被升级——接受这个极小的风险， +// 换取"内核能更新 skill"这个基本能力。 +func builtinSkillOutdated(dir string, embedded []byte) bool { + cur, err := os.ReadFile(filepath.Join(dir, skillFileName)) + if err != nil { + return false + } + diskVer := skillVersionOf(string(cur)) + embVer := skillVersionOf(string(embedded)) + if diskVer == "" || embVer == "" { + return false + } + return compareVersion(diskVer, embVer) < 0 +} + +// skillVersionOf 从 SKILL.md 的 frontmatter 里取 version。 +// +// 只认 frontmatter 块（首个 --- 到闭合 ---），不扫正文 —— 正文里的 +// `version:` 可能是示例代码，误读会让判据做出错误决定。 +func skillVersionOf(content string) string { + lines := strings.Split(content, "\n") + in := false + for i, line := range lines { + t := strings.TrimSpace(line) + if i == 0 && t == "---" { + in = true + continue + } + if !in { + continue + } + if t == "---" { + break // frontmatter 结束 + } + if strings.HasPrefix(strings.ToLower(t), "version:") { + v := strings.TrimSpace(t[len("version:"):]) + v = strings.Trim(v, "\"'") + return v + } + } + return "" +} + +// compareVersion 按点分段数值比较，返回 -1 / 0 / 1。 +// +// 非数值段按字符串比较（例如 "1.0.0-beta"），空段视为 0。 +// 不做语义化版本的完整解析 —— 这里只需要"新是否旧"，越简单越不会错。 +func compareVersion(a, b string) int { + as := strings.Split(a, ".") + bs := strings.Split(b, ".") + n := len(as) + if len(bs) > n { + n = len(bs) + } + for i := 0; i < n; i++ { + // ★ 缺失段必须视为 "0"，不能留空串。 + // + // 判据实测抓到过：留空串时 Atoi("") 失败 ⇒ 落到字符串比较 + // ⇒ `"" < "0"` ⇒ "1.0" 被判成低于 "1.0.0"（错误，二者等价）。 + // 后果是版本号少写一段的 skill 会被无谓地重复升级。 + av, bv := "0", "0" + if i < len(as) && as[i] != "" { + av = as[i] + } + if i < len(bs) && bs[i] != "" { + bv = bs[i] + } + ai, aerr := strconv.Atoi(av) + bi, berr := strconv.Atoi(bv) + if aerr == nil && berr == nil { + if ai != bi { + if ai < bi { + return -1 + } + return 1 + } + continue + } + // 非数值段：字符串比较 + if av != bv { + if av < bv { + return -1 + } + return 1 + } + } + return 0 +} diff --git a/internal/plugins/skillmgr/builtin/kernel-plugins/SKILL.md b/internal/plugins/skillmgr/builtin/kernel-plugins/SKILL.md index cd62f9e9..3c9682e0 100644 --- a/internal/plugins/skillmgr/builtin/kernel-plugins/SKILL.md +++ b/internal/plugins/skillmgr/builtin/kernel-plugins/SKILL.md @@ -1,7 +1,7 @@ --- name: kernel-plugins description: Tools are per-plugin; on-demand plugins need plugin_start before their tools. -version: 1.0.0 +version: 1.1.0 --- # Plugin capabilities @@ -15,3 +15,23 @@ Tools are grouped per plugin and registered at runtime. - `plugin_status` shows what is running; `plugin_restart` / `plugin_stop` manage lifecycle. - The matching skill (same name as the plugin, or a related one) states whether the plugin is resident or on-demand, and how to use it once started. + +## Reading a skill without burning context + +`skill_info(name)` returns the whole document for short skills, but for long ones it returns a +**table of contents + the first 30 lines**. Each TOC line is annotated `(N lines)` — use it to +judge the cost before committing. + +To read one part only, pass `section`: + +``` +skill_info("commondev-git", section="14") # by number +skill_info("commondev-git", section="常见操作") # by title, substring, case-insensitive +``` + +- Top-level sections return their **whole subtree** (asking for `Setup` gives all its `###` + subsections, not just the heading line). +- `#` prefixes are tolerated: `section="## Setup"` works. +- A miss is not silent — the error lists every available section, so one retry is enough. +- Section boundaries respect fenced code blocks: a `#` comment inside a ```bash block is never + treated as a heading. diff --git a/internal/plugins/skillmgr/builtin_sync_test.go b/internal/plugins/skillmgr/builtin_sync_test.go index 77cd545c..4edc9e1c 100644 --- a/internal/plugins/skillmgr/builtin_sync_test.go +++ b/internal/plugins/skillmgr/builtin_sync_test.go @@ -29,6 +29,7 @@ package skillmgr import ( "os" "path/filepath" + "strings" "testing" ) @@ -176,3 +177,99 @@ func TestMaterializeBuiltinSkills_Idempotent(t *testing.T) { t.Fatal("内容未变却重写了文件（mtime 变了）") } } + +// ───────────────────────────────────────────────────────────── +// 没有 stamp 的**内核旧版**必须能升级（2026-11-04 修的实测缺陷） +// +// 背景：stamp 机制由 387ae94 引入，而生产机上 kernel-plugins 是更早的 +// 内核写的（mtime 2026-10-26），没有 .builtin-sha256 ⇒ unmodified 恒为 +// false ⇒ 这份 skill **永不更新**。后果：内核改进了 skill 文档，已部署 +// 的机器永远拿不到 —— 而它正是教模型怎么用工具的文档。 +// +// 与 TestMaterializeBuiltinSkills_NoStampIsConservative 的关系： +// 那条判据守"来历不明的旧内容别覆盖"，这条守"内核旧版要升级"。 +// 两者的分界线是 frontmatter 里的 version：版本更高 = 内核发布了新版。 +// ───────────────────────────────────────────────────────────── + +// TestMaterializeBuiltinSkills_UpgradesStaleKernelVersion 钉住升级路径。 +// +// 变异：把 builtinSkillOutdated 的返回值改成恒 false ⇒ 本例红。 +func TestMaterializeBuiltinSkills_UpgradesStaleKernelVersion(t *testing.T) { + dir := writeBuiltinFixture(t) + + // 模拟"没有 stamp 的内核旧版"：version 刻意写低。 + stale := "---\nname: kernel-memory\ndescription: old\nversion: 0.0.1\n---\n\n# 旧内容\n" + putSkill(t, dir, stale, false) + + materializeBuiltinSkills(dir) + + got, err := os.ReadFile(filepath.Join(dir, "kernel-memory", skillFileName)) + if err != nil { + t.Fatalf("读回: %v", err) + } + if strings.Contains(string(got), "旧内容") { + t.Fatalf("★ 没有 stamp 的内核旧版（version 0.0.1）没被升级 ——\n"+ + "这正是生产上 kernel-plugins 永不更新的原因。\n实际内容: %q", string(got)) + } + if !strings.Contains(string(got), "version: 1.") && !strings.Contains(string(got), "version: 2.") { + t.Logf("升级后版本: %s", skillVersionOf(string(got))) + } + // 升级后必须补上 stamp，否则下次又会走"无 stamp"分支 + if _, err := os.Stat(filepath.Join(dir, "kernel-memory", builtinStampName)); err != nil { + t.Fatalf("★ 升级后没写 stamp：%v（下次仍会被当成来历不明）", err) + } +} + +// TestMaterializeBuiltinSkills_KeepsHigherUserVersion 钉住反向： +// 用户把 version 改得比内嵌更高时，**必须保留**（那是人明确要的版本）。 +// +// 变异：把 compareVersion < 0 改成 <= 0 ⇒ 相等也升级 ⇒ 本例如改高则红。 +func TestMaterializeBuiltinSkills_KeepsHigherUserVersion(t *testing.T) { + dir := writeBuiltinFixture(t) + custom := "---\nname: kernel-memory\ndescription: mine\nversion: 99.0.0\n---\n\n# 我的定制版\n" + putSkill(t, dir, custom, false) + + materializeBuiltinSkills(dir) + + got, err := os.ReadFile(filepath.Join(dir, "kernel-memory", skillFileName)) + if err != nil { + t.Fatalf("读回: %v", err) + } + if !strings.Contains(string(got), "我的定制版") { + t.Fatalf("★ 用户把 version 改成 99.0.0 后仍被覆盖（现在 %q）——\n"+ + "version 更高意味着人明确要这个版本，必须保留", string(got)) + } +} + +// TestCompareVersion 钉住版本比较本身（判据的基础）。 +func TestCompareVersion(t *testing.T) { + cases := []struct { + a, b string + want int + }{ + {"1.0.0", "1.0.0", 0}, + {"1.0.0", "1.0.1", -1}, + {"1.1.0", "1.0.0", 1}, + {"1.0", "1.0.0", 0}, // 缺段视为 0 + {"2", "10", -1}, // 数值比较，不是字符串（"2" > "10" 是字符串陷阱） + {"1.0.0", "1.0.0-beta", -1}, + } + for _, c := range cases { + if got := compareVersion(c.a, c.b); got != c.want { + t.Errorf("compareVersion(%q,%q)=%d，期望 %d", c.a, c.b, got, c.want) + } + } +} + +// TestSkillVersionOfOnlyReadsFrontmatter 钉住版本解析只看 frontmatter。 +// +// 正文里的 `version:` 可能是示例代码，误读会让升级判据做出错误决定。 +func TestSkillVersionOfOnlyReadsFrontmatter(t *testing.T) { + content := "---\nname: x\nversion: 1.2.3\n---\n\n正文\n\n```yaml\nversion: 9.9.9\n```\n" + if got := skillVersionOf(content); got != "1.2.3" { + t.Fatalf("★ 解析到 %q，期望 1.2.3（正文里的 version 不应被读）", got) + } + if got := skillVersionOf("没有 frontmatter 的文档\nversion: 5.0.0\n"); got != "" { + t.Fatalf("★ 无 frontmatter 却解析出 %q，期望空", got) + } +}