Skip to content

fix(skills): preserve update tracking metadata - #1824

Open
xu91102 wants to merge 1 commit into
larksuite:mainfrom
xu91102:fix/skills-update-tracking
Open

fix(skills): preserve update tracking metadata#1824
xu91102 wants to merge 1 commit into
larksuite:mainfrom
xu91102:fix/skills-update-tracking

Conversation

@xu91102

@xu91102 xu91102 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

背景

lark-cli update 同步官方 skills 时优先通过 https://open.feishu.cn well-known 源安装。该路径会让 skills 的 lock metadata 记录为 well-known,导致后续 npx skills update -g -y 将 Lark skills 标记为 Well-known skill 并跳过自动检查。

Fixes #1125

核心改动

  • 将 skills 安装同步的主源改为 larksuite/cli,让通用 skills update 能保留 repo tracking metadata。
  • 保留 https://open.feishu.cn 作为安装失败时的 fallback,不影响原有可用性。
  • 增加回归测试覆盖主源选择和 fallback 顺序。

验证

  • GOCACHE=$PWD/.cache/go-build GOPROXY=https://goproxy.cn,direct go test ./internal/selfupdate -run 'TestInstallSkillsPreferRepoSourceForUpdateTracking|TestInstallSkillsFallBackToWellKnownSource|TestSkillsCommandsUseExpectedArgs' -count=1:通过
  • GOCACHE=$PWD/.cache/go-build GOPROXY=https://goproxy.cn,direct go test ./internal/selfupdate ./internal/skillscheck -count=1:通过
  • GOCACHE=$PWD/.cache/go-build GOPROXY=https://goproxy.cn,direct go test ./cmd/update -run '^(TestUpdatePnpm_JSON|TestUpdatePnpm_Human|TestUpdatePnpm_InstallError_JSON)$' -count=1 -timeout=30s:通过
  • GOCACHE=$PWD/.cache/go-build GOPROXY=https://goproxy.cn,direct go build -o ./lark-cli .:通过
  • git diff --check:通过
  • gofmt -l internal/selfupdate/updater.go internal/selfupdate/updater_test.go:通过,无输出
  • rg -n '^(<<<<<<< .+|=======$|>>>>>>> .+)$' --glob '!vendor/**' --glob '!node_modules/**' .:通过,无冲突标记

未运行:

  • go test ./cmd/update -count=1:本地会命中需要真实 npx skills add ... 的长路径;已改跑本次 update skills sync 相关的 mock 单测。

风险与回滚

风险较低,改动只调整 skills 安装源优先级。若 larksuite/cli 源不可用,仍会 fallback 到 https://open.feishu.cn。回滚此提交即可恢复原顺序。

Summary by CodeRabbit

  • Bug Fixes
    • Improved skills installation reliability by trying the preferred source first and automatically falling back to an alternate source if needed.
    • This applies to both installing a specific skill and installing all skills, helping reduce setup failures.

@xu91102
xu91102 requested a review from liangshuo-1 as a code owner July 9, 2026 06:05
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 70f990f5-7b85-4281-8877-b36eefa27bdf

📥 Commits

Reviewing files that changed from the base of the PR and between 74d8458 and eab4847.

📒 Files selected for processing (2)
  • internal/selfupdate/updater.go
  • internal/selfupdate/updater_test.go

📝 Walkthrough

Walkthrough

InstallSkill and InstallAllSkills in the Updater now attempt the larksuite/cli source first, falling back to the open.feishu.cn source only if the primary attempt fails. Two new tests validate this preferred order and the fallback behavior when the primary source call fails.

Changes

Skills install source order

Layer / File(s) Summary
Swap install/add fallback order
internal/selfupdate/updater.go
InstallSkill and InstallAllSkills now try larksuite/cli first via runSkillsInstall/runSkillsAdd, falling back to https://open.feishu.cn only on error.
Fallback and preference tests
internal/selfupdate/updater_test.go
New tests verify repo-source calls are issued first for both scoped and global installs, and that a simulated primary-source failure triggers fallback to the well-known Feishu URL source.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • larksuite/cli#1042: Touches the same InstallSkill/InstallAllSkills APIs in internal/selfupdate/updater.go used by the update sync flow.

Suggested labels: bug

Suggested reviewers: MaxHuang22, liangshuo-1

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: preserving skills update-tracking metadata.
Description check ✅ Passed The description covers background, changes, verification, and the linked issue, though it doesn't follow the template headings exactly.
Linked Issues check ✅ Passed The code changes and tests address #1125 by preferring a repo source to keep tracking metadata and falling back to the well-known source.
Out of Scope Changes check ✅ Passed No clearly unrelated code changes are present; the diff stays focused on skills install source selection and regression tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lark-cli update makes npx skills update skip Lark skills as Well-known skill

1 participant