diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index 5cfe52477..54e6cfe62 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -61,6 +61,8 @@ func successfulSkillsCommand() func(args ...string) *selfupdate.NpmResult { switch strings.Join(args, " ") { case "-y skills add https://open.feishu.cn --list": r.Stdout.WriteString("Available Skills\n │ lark-calendar\n │ lark-mail\n") + case "-y skills ls -g --json": + r.Stdout.WriteString(`[{"name":"lark-calendar","path":"/tmp/lark-calendar","scope":"global","agents":["Codex"]},{"name":"custom-skill","path":"/tmp/custom-skill","scope":"global","agents":["Codex"]}]`) case "-y skills ls -g": r.Stdout.WriteString("Global Skills\nlark-calendar /tmp/lark-calendar\ncustom-skill /tmp/custom-skill\n") default: diff --git a/internal/selfupdate/updater.go b/internal/selfupdate/updater.go index 8cba0b7d3..aa39bfaf2 100644 --- a/internal/selfupdate/updater.go +++ b/internal/selfupdate/updater.go @@ -165,6 +165,10 @@ func (u *Updater) ListGlobalSkills() *NpmResult { return u.runSkillsListGlobal() } +func (u *Updater) ListGlobalSkillsJSON() *NpmResult { + return u.runSkillsCommand("-y", "skills", "ls", "-g", "--json") +} + func (u *Updater) InstallSkill(nameList []string) *NpmResult { r := u.runSkillsInstall("https://open.feishu.cn", nameList) if r.Err != nil { diff --git a/internal/selfupdate/updater_test.go b/internal/selfupdate/updater_test.go index 458f3f79b..b01a15840 100644 --- a/internal/selfupdate/updater_test.go +++ b/internal/selfupdate/updater_test.go @@ -188,6 +188,13 @@ func TestSkillsCommandsUseExpectedArgs(t *testing.T) { }, want: "-y skills ls -g", }, + { + name: "list global json", + run: func(u *Updater) *NpmResult { + return u.ListGlobalSkillsJSON() + }, + want: "-y skills ls -g --json", + }, { name: "install skill primary", run: func(u *Updater) *NpmResult { diff --git a/internal/skillscheck/sync.go b/internal/skillscheck/sync.go index 3239131b4..48e4de9ba 100644 --- a/internal/skillscheck/sync.go +++ b/internal/skillscheck/sync.go @@ -4,6 +4,7 @@ package skillscheck import ( + "encoding/json" "fmt" "regexp" "sort" @@ -57,6 +58,28 @@ func ParseSkillsList(text string) []string { return nil } +func ParseGlobalSkillsJSON(text string) []string { + type globalSkill struct { + Name string `json:"name"` + } + + var skills []globalSkill + if err := json.Unmarshal([]byte(text), &skills); err != nil { + return nil + } + + seen := map[string]bool{} + for _, skill := range skills { + candidate := strings.TrimSpace(skill.Name) + if candidate == "" || !skillNamePattern.MatchString(candidate) { + continue + } + seen[candidate] = true + } + + return sortedKeys(seen) +} + // parseGlobalSkillsList parses the output of "npx -y skills ls -g" func parseGlobalSkillsList(lines []string) []string { seen := map[string]bool{} @@ -77,8 +100,11 @@ func parseGlobalSkillsList(lines []string) []string { continue } - // Skip indented lines (Agents: ...) - if strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t") { + if strings.HasPrefix(trimmed, "Agents:") { + continue + } + + if isGlobalSkillsSectionHeader(trimmed) { continue } @@ -91,21 +117,24 @@ func parseGlobalSkillsList(lines []string) []string { candidate := parts[0] // Validate and add - if candidate == "" || strings.Contains(candidate, " ") || strings.HasSuffix(candidate, ":") { + if candidate == "" || !skillNamePattern.MatchString(candidate) { continue } - if !skillNamePattern.MatchString(candidate) { - continue - } - if at := strings.Index(candidate, "@"); at > 0 { - candidate = candidate[:at] - } seen[candidate] = true } return sortedKeys(seen) } +func isGlobalSkillsSectionHeader(line string) bool { + switch line { + case "General", "Project", "Local": + return true + default: + return false + } +} + // parseOfficialSkillsList parses the output of "npx -y skills add ... --list" func parseOfficialSkillsList(lines []string) []string { seen := map[string]bool{} @@ -195,6 +224,7 @@ func PlanSync(input SyncInput) SyncPlan { type SkillsRunner interface { ListOfficialSkills() *selfupdate.NpmResult + ListGlobalSkillsJSON() *selfupdate.NpmResult ListGlobalSkills() *selfupdate.NpmResult InstallSkill(nameList []string) *selfupdate.NpmResult InstallAllSkills() *selfupdate.NpmResult @@ -239,10 +269,9 @@ func SyncSkills(opts SyncOptions) *SyncResult { } // --- Step 2: List local (installed) skills --- - local := []string{} - localResult := opts.Runner.ListGlobalSkills() - if localResult != nil && localResult.Err == nil { - local = ParseSkillsList(localResult.Stdout.String()) + local, ok := listLocalSkills(opts.Runner) + if !ok { + return fallbackFullInstall(opts, "local skills list failed or parsed as empty", official) } // --- Step 3: Read previous state --- @@ -298,6 +327,24 @@ func SyncSkills(opts SyncOptions) *SyncResult { return result } +func listLocalSkills(runner SkillsRunner) ([]string, bool) { + jsonResult := runner.ListGlobalSkillsJSON() + if jsonResult != nil && jsonResult.Err == nil { + if local := ParseGlobalSkillsJSON(jsonResult.Stdout.String()); len(local) > 0 { + return local, true + } + } + + textResult := runner.ListGlobalSkills() + if textResult != nil && textResult.Err == nil { + if local := ParseSkillsList(textResult.Stdout.String()); len(local) > 0 { + return local, true + } + } + + return nil, false +} + // fallbackFullInstall performs a full skills install (npx -y skills add -g -y) // when incremental sync is not possible. On success it writes a state file so that // subsequent syncs can use incremental mode. When official is non-nil the state diff --git a/internal/skillscheck/sync_test.go b/internal/skillscheck/sync_test.go index 059bc656f..e0e9ee249 100644 --- a/internal/skillscheck/sync_test.go +++ b/internal/skillscheck/sync_test.go @@ -67,6 +67,49 @@ func TestParseGlobalSkillsListWithANSI(t *testing.T) { } } +func TestParseGlobalSkillsListWithIndentedGroupedRows(t *testing.T) { + input := `Global Skills + +General + lark-apps ~/.agents/skills/lark-apps + lark-base ~/.agents/skills/lark-base +` + got := ParseSkillsList(input) + want := []string{"lark-apps", "lark-base"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("ParseSkillsList() (indented Global Skills) = %#v, want %#v", got, want) + } +} + +func TestParseGlobalSkillsJSON(t *testing.T) { + input := `[ + {"name":"lark-calendar","path":"/Users/example/.agents/skills/lark-calendar","scope":"global","agents":["Codex"]}, + {"name":"lark-mail@1.2.3","path":"/Users/example/.agents/skills/lark-mail","scope":"global","agents":["Codex"]}, + {"name":"lark-calendar","path":"/Users/example/.agents/skills/lark-calendar","scope":"global","agents":["Codex"]}, + {"name":" lark-base ","path":"/Users/example/.agents/skills/lark-base","scope":"global","agents":["Codex"]}, + {"name":""}, + {"name":" "}, + {"name":"bad skill"} +]` + got := ParseGlobalSkillsJSON(input) + want := []string{"lark-base", "lark-calendar", "lark-mail@1.2.3"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("ParseGlobalSkillsJSON() = %#v, want %#v", got, want) + } +} + +func TestParseGlobalSkillsJSONInvalidOrUnsupported(t *testing.T) { + for _, input := range []string{ + `not json`, + `{"name":"lark-calendar"}`, + `[]`, + } { + if got := ParseGlobalSkillsJSON(input); len(got) != 0 { + t.Fatalf("ParseGlobalSkillsJSON(%q) = %#v, want empty", input, got) + } + } +} + func TestPlanNormal_WithReadableStatePreservesDeletedAndAddsNew(t *testing.T) { previous := &SkillsState{OfficialSkills: []string{"lark-calendar", "lark-mail"}} got := PlanSync(SyncInput{ @@ -113,14 +156,18 @@ func TestPlanForceRestoresAllOfficial(t *testing.T) { } type fakeSkillsRunner struct { - officialOut string - globalOut string - officialErr error - globalErr error - installErr error - installAllErr error - installed [][]string - installedAll int + officialOut string + globalJSONOut string + globalOut string + officialErr error + globalJSONErr error + globalErr error + installErr error + installAllErr error + installed [][]string + installedAll int + listedGlobalJSON int + listedGlobalText int } func officialSkillsOutput(names ...string) string { @@ -146,6 +193,19 @@ func globalSkillsOutput(names ...string) string { return b.String() } +func globalSkillsJSONOutput(names ...string) string { + var b strings.Builder + b.WriteString("[") + for i, name := range names { + if i > 0 { + b.WriteString(",") + } + fmt.Fprintf(&b, `{"name":%q,"path":"/Users/example/.agents/skills/%s","scope":"global","agents":["Codex"]}`, name, name) + } + b.WriteString("]") + return b.String() +} + func (f *fakeSkillsRunner) ListOfficialSkills() *selfupdate.NpmResult { r := &selfupdate.NpmResult{} r.Stdout.WriteString(f.officialOut) @@ -153,7 +213,16 @@ func (f *fakeSkillsRunner) ListOfficialSkills() *selfupdate.NpmResult { return r } +func (f *fakeSkillsRunner) ListGlobalSkillsJSON() *selfupdate.NpmResult { + f.listedGlobalJSON++ + r := &selfupdate.NpmResult{} + r.Stdout.WriteString(f.globalJSONOut) + r.Err = f.globalJSONErr + return r +} + func (f *fakeSkillsRunner) ListGlobalSkills() *selfupdate.NpmResult { + f.listedGlobalText++ r := &selfupdate.NpmResult{} r.Stdout.WriteString(f.globalOut) r.Err = f.globalErr @@ -186,8 +255,9 @@ func TestSyncSkills_WritesStateAndDoesNotWriteStamp(t *testing.T) { } runner := &fakeSkillsRunner{ - officialOut: officialSkillsOutput("lark-calendar", "lark-mail", "lark-new"), - globalOut: globalSkillsOutput("lark-calendar", "lark-custom"), + officialOut: officialSkillsOutput("lark-calendar", "lark-mail", "lark-new"), + globalJSONOut: globalSkillsJSONOutput("lark-calendar", "lark-custom"), + globalOut: globalSkillsOutput("lark-mail"), } result := SyncSkills(SyncOptions{ Version: "1.0.33", @@ -199,6 +269,12 @@ func TestSyncSkills_WritesStateAndDoesNotWriteStamp(t *testing.T) { t.Fatalf("SyncSkills() err = %v, want nil", result.Err) } assertStrings(t, runner.installed[0], []string{"lark-calendar", "lark-new"}) + if runner.listedGlobalJSON != 1 { + t.Fatalf("listedGlobalJSON = %d, want 1", runner.listedGlobalJSON) + } + if runner.listedGlobalText != 0 { + t.Fatalf("listedGlobalText = %d, want 0 when JSON list succeeds", runner.listedGlobalText) + } state, readable, err := ReadState() if err != nil || !readable { @@ -262,47 +338,73 @@ func TestSyncSkills_ListOfficialFailureAndFullInstallFails(t *testing.T) { } } -func TestSyncSkills_GlobalListFailureDegradesToColdStart(t *testing.T) { +func TestSyncSkills_GlobalJSONFailureFallsBackToTextList(t *testing.T) { dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) runner := &fakeSkillsRunner{ - officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), - globalErr: fmt.Errorf("global list failed"), + officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalJSONErr: fmt.Errorf("json list failed"), + globalOut: globalSkillsOutput("lark-calendar"), } result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) if result.Err != nil { - t.Fatalf("SyncSkills() err = %v, want nil (degraded to cold start)", result.Err) + t.Fatalf("SyncSkills() err = %v, want nil", result.Err) } if result.Action != "synced" { t.Fatalf("SyncSkills() action = %q, want synced", result.Action) } assertStrings(t, result.Updated, []string{"lark-calendar", "lark-mail"}) - assertStrings(t, result.SkippedDeleted, []string{}) + if runner.listedGlobalJSON != 1 || runner.listedGlobalText != 1 { + t.Fatalf("listed JSON/text = %d/%d, want 1/1", runner.listedGlobalJSON, runner.listedGlobalText) + } + if runner.installedAll != 0 { + t.Fatalf("installedAll = %d, want 0", runner.installedAll) + } } -func TestSyncSkills_ParseEmptyGlobalListWithNonEmptyStdoutDegradesToColdStart(t *testing.T) { +func TestSyncSkills_LocalListsFailureFallsBackToFullInstall(t *testing.T) { dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) runner := &fakeSkillsRunner{ - officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), - globalOut: "Some unrecognized output format\n", + officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalJSONErr: fmt.Errorf("json list failed with /Users/example/.agents/skills/lark-calendar agents Codex"), + globalErr: fmt.Errorf("text list failed with /Users/example/.agents/skills/lark-mail agents Codex"), } result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) - if result.Err != nil { - t.Fatalf("SyncSkills() err = %v, want nil (degraded to cold start)", result.Err) + if result.Action != "fallback_synced" { + t.Fatalf("SyncSkills() action = %q, want fallback_synced", result.Action) } - if result.Action != "synced" { - t.Fatalf("SyncSkills() action = %q, want synced", result.Action) + if len(runner.installed) != 0 { + t.Fatalf("installed = %#v, want no incremental installs", runner.installed) } - assertStrings(t, result.Updated, []string{"lark-calendar", "lark-mail"}) - assertStrings(t, result.SkippedDeleted, []string{}) - if runner.installedAll != 0 { - t.Fatalf("installedAll = %d, want 0 (no fallback)", runner.installedAll) + if runner.installedAll != 1 { + t.Fatalf("installedAll = %d, want 1", runner.installedAll) } - if len(runner.installed) != 1 { - t.Fatalf("installed = %d calls, want 1 (incremental)", len(runner.installed)) + if strings.Contains(result.Detail, "/Users/example") || strings.Contains(result.Detail, "agents") { + t.Fatalf("SyncSkills() detail leaks local command output: %q", result.Detail) + } +} + +func TestSyncSkills_ParseEmptyLocalListsFallBackToFullInstall(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + runner := &fakeSkillsRunner{ + officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalJSONOut: `[]`, + globalOut: "Some unrecognized output format\n", + } + + result := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner, Now: time.Now}) + if result.Action != "fallback_synced" { + t.Fatalf("SyncSkills() action = %q, want fallback_synced", result.Action) + } + if len(runner.installed) != 0 { + t.Fatalf("installed = %#v, want no incremental installs", runner.installed) + } + if runner.installedAll != 1 { + t.Fatalf("installedAll = %d, want 1", runner.installedAll) } } @@ -344,6 +446,7 @@ func TestSyncSkills_InstallFailureFallsBackToFullInstall(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) runner := &fakeSkillsRunner{ officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalJSONOut: globalSkillsJSONOutput("lark-calendar", "lark-mail"), globalOut: globalSkillsOutput("lark-calendar", "lark-mail"), installErr: fmt.Errorf("incremental boom"), installAllErr: nil, @@ -375,6 +478,7 @@ func TestSyncSkills_InstallFailureAndFullInstallFails(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) runner := &fakeSkillsRunner{ officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalJSONOut: globalSkillsJSONOutput("lark-calendar", "lark-mail"), globalOut: globalSkillsOutput("lark-calendar", "lark-mail"), installErr: fmt.Errorf("incremental boom"), installAllErr: fmt.Errorf("full install boom"), @@ -473,6 +577,7 @@ func TestSyncSkills_FallbackWithKnownOfficialWritesFullState(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) runner := &fakeSkillsRunner{ officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalJSONOut: globalSkillsJSONOutput("lark-calendar", "lark-mail"), globalOut: globalSkillsOutput("lark-calendar", "lark-mail"), installErr: fmt.Errorf("incremental boom"), installAllErr: nil, @@ -497,6 +602,7 @@ func TestSyncSkills_FallbackResultContainsMetadata(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) runner := &fakeSkillsRunner{ officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalJSONOut: globalSkillsJSONOutput("lark-calendar", "lark-mail"), globalOut: globalSkillsOutput("lark-calendar", "lark-mail"), installErr: fmt.Errorf("incremental boom"), installAllErr: nil, @@ -537,8 +643,9 @@ func TestSyncSkills_FallbackBreaksDegradationLoop(t *testing.T) { } runner2 := &fakeSkillsRunner{ - officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), - globalOut: globalSkillsOutput("lark-calendar", "lark-mail"), + officialOut: officialSkillsOutput("lark-calendar", "lark-mail"), + globalJSONOut: globalSkillsJSONOutput("lark-calendar", "lark-mail"), + globalOut: globalSkillsOutput("lark-calendar", "lark-mail"), } result2 := SyncSkills(SyncOptions{Version: "1.0.33", Runner: runner2, Now: time.Now}) if result2.Action != "synced" {