fix: parse grouped skills list output

[nguyenngothuong]

Summary

Fix the incremental skills sync local-skill detection when npx skills ls -g prints grouped, indented human-readable output. The parser now recognizes indented skill rows without treating category headings as skills.

Changes

• Allow global skills parser to parse indented skill rows that include a skill path.
• Ignore category headings, including multi-word headings, by requiring the second token to look like a path.
• Add regression coverage for the grouped output shape reported in lark-cli update incremental skills sync does not detect installed skills from grouped skills ls output #1186.

Test Plan

• Unit tests pass: go test ./internal/skillscheck -count=1
• Unit tests pass: go test ./cmd/update -count=1
• Unit tests pass: go test ./internal/skillscheck ./cmd/update -count=1
• Manual local verification confirms the lark-cli update flow works as expected

Related Issues

• Fixes lark-cli update incremental skills sync does not detect installed skills from grouped skills ls output #1186

Summary by CodeRabbit

• Bug Fixes

  • Improved skill extraction to correctly parse grouped and indented skills from the Global Skills list, ensuring all skills are properly identified and processed.

• Tests

  • Added test coverage for skill extraction with grouped indented sections.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0e6de5eb-34a3-4b80-a01c-dacb553c8c27

📥 Commits

Reviewing files that changed from the base of the PR and between 99e314f and e31109f.

📒 Files selected for processing (2)

• internal/skillscheck/sync.go
• internal/skillscheck/sync_test.go

---

📝 Walkthrough

Walkthrough

Updated parseGlobalSkillsList to handle indented skill listings by validating field count and recognizing skill paths using a new looksLikeSkillPath predicate, replacing prior indentation-based filtering. Added test coverage for grouped/indented output.

Changes

Grouped Skills Output Parsing

Layer / File(s)	Summary
Path validation for grouped indented skills internal/skillscheck/sync.go, internal/skillscheck/sync_test.go	looksLikeSkillPath predicate recognizes common skill path patterns (home, absolute, relative, Windows drives, forward-slashes). parseGlobalSkillsList now requires at least two fields and validates the second field as a path pattern instead of skipping indented lines. New test TestParseGlobalSkillsListWithGroupedIndentedSkills verifies parsing of grouped/categorized skill output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• larksuite/cli#1042: Introduces the overall skillscheck sync/parsing system that this PR enhances to handle grouped output formats.

Suggested labels

bug

Poem

🐰 Indented skills once slipped away unnoticed,
Now grouped by category, they're nicely noticed!
With paths recognized and validation tight,
The parser hops through grouped lists just right. 🌱

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: parse grouped skills list output' directly and concisely describes the main change of updating the skills parser to handle grouped, indented output.
Description check	✅ Passed	The description includes all required sections: Summary, Changes, Test Plan (with checkmarks showing unit tests pass), and Related Issues with fix reference #1186.
Linked Issues check	✅ Passed	The code changes directly address issue #1186 by allowing the parser to handle indented skill rows with path validation and adding regression test coverage for grouped output.
Out of Scope Changes check	✅ Passed	All changes are within scope: parser logic update to handle grouped output, new helper function for path validation, and regression test for the specific issue reported.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[nguyenngothuong]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[zhangheng023]

Hi @nguyenngothuong , thank you for the contribution and for helping improve the skills sync reliability.

I reviewed this PR, and it addresses the same issue as #1186 by improving the parser for grouped and indented skills ls -g output.
We have now fixed this problem in #1251, which has been merged.

The merged fix takes a slightly broader approach:

1. It first tries to detect locally installed skills using structured JSON output:

   npx -y skills ls -g --json

2. If JSON output is unavailable or returns an empty result, it falls back to the existing human-readable text parser.

3. The text parser fallback was also updated to support grouped and indented skill rows, covering the case handled by this PR.

4. The existing final fallback behavior is preserved as well.

Because #1251 already covers the issue this PR is solving, I’m going to close this PR as superseded to avoid maintaining duplicate
fixes.

Thanks again for taking the time to investigate this and submit a fix. We really appreciate the contribution!

[nguyenngothuong]

ok got it