feat: add incremental skills sync (#965)"

[zhangheng023]

Summary

Changes

• Change 1
• Change 2

Test Plan

• Unit tests pass
• Manual local verification confirms the lark xxx command works as expected

Related Issues

• None

Summary by CodeRabbit

Release Notes

• Refactor
  • Updated skills tracking mechanism to use a simpler persistence approach
  • Streamlined skills installation and update workflow
  • Simplified the AI skills setup process in the installer wizard

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR replaces JSON state-based skills synchronization with a lightweight stamp-file mechanism. The migration removes the SkillsState/SyncResult infrastructure and the SyncSkills orchestration, introducing simple version-stamp persistence under the config directory. The update command's three sync paths now use stamp deduplication to skip redundant npx invocations when versions match, and the install wizard is simplified to check-then-install logic without state-aware planning.

Changes

Skills sync state → stamp migration

Layer / File(s)	Summary
Stamp persistence foundation internal/skillscheck/stamp.go, internal/skillscheck/stamp_test.go	New module implementing ReadStamp() and WriteStamp() for simple version-string persistence under the base config directory, with tests covering missing-file handling, trailing-newline tolerance, directory creation, and I/O errors.
Skills check refactoring to use stamps internal/skillscheck/check.go, internal/skillscheck/check_test.go, internal/skillscheck/notice.go	Init() refactored to compare binary version against persisted skills.stamp instead of skills-state.json, implementing fail-closed error handling, cold-start detection, and drift-notice generation only when stamp differs from current version. Tests updated to verify in-sync, drift, cold-start, and error-path behaviors.
Selfupdate module refactoring internal/selfupdate/updater.go, internal/selfupdate/updater_test.go	Removes SkillsCommandOverride testing hook and public skills-management APIs (ListOfficialSkills, ListGlobalSkills, InstallSkill), replacing them with simplified runSkillsAdd(source) that directly runs npx -y skills add <source> -g -y with timeout handling and returns NpmResult.
Update command migration to stamp-based skills sync cmd/update/update.go	All three update paths (already-up-to-date, manual-required, npm-update) switch to runSkillsAndStamp(), implementing stamp-based deduplication (skip npx if stamped version matches target), atomic stamp writing on success with warning-only failure semantics, and direct ReadStamp() for --check JSON skills_status reporting. Stale-file cleanup moved unconditionally.
Update command test infrastructure and stamp isolation cmd/update/update_test.go (infrastructure)	Test helper mockDetectAndNpm refactored to accept explicit skills override functions via SkillsUpdateOverride (replacing removed SkillsCommandOverride), adds newTestIO() helper, and implements config-dir isolation via t.TempDir() to prevent real stamp-file mutations during testing.
Update command tests for stamp behavior and runSkillsAndStamp cmd/update/update_test.go (new test coverage)	New test cases verify runSkillsAndStamp deduplication (stamp-hit vs forced), success stamp writing, stamp preservation on failure, stderr warning on WriteStamp failure (filesystem error), and ensures skills sync skips after verification failures and under --check. Existing tests updated to use SkillsUpdateOverride and assert ReadStamp() matches expected versions.
Integration tests updated for stamp persistence cmd/root_integration_test.go	Root integration tests now use skillscheck.WriteStamp() instead of skillscheck.WriteState() for cold-start, in-sync, drift, and both-update-and-skills scenarios to drive expected notice composition.
Install wizard simplification scripts/install-wizard.js	Global install step now returns boolean (upgrade needed) instead of version string; skills install logic checks if skills already exist and conditionally skips or installs via npx skills add with primary and fallback repos, removing semver-aware state-based sync planning. Main orchestration no longer threads cli-version between steps.

Sequence Diagram(s)

sequenceDiagram
  participant Init as Init(currentVersion)
  participant ReadStamp as ReadStamp()
  participant File as skills.stamp
  participant Notice as StaleNotice
  
  Init->>ReadStamp: read persisted stamp
  ReadStamp->>File: lookup stamp file
  alt Missing stamp (cold start)
    File-->>ReadStamp: ENOENT
    ReadStamp-->>Init: empty string, nil error
    Init->>Init: no notice
  else Non-ENOENT I/O error
    File-->>ReadStamp: I/O error
    ReadStamp-->>Init: error
    Init->>Init: fail-closed: no notice
  else Empty stamp content
    File-->>ReadStamp: empty string
    ReadStamp-->>Init: empty string, nil
    Init->>Init: no notice
  else Stamp matches current
    File-->>ReadStamp: version string
    ReadStamp-->>Init: version matches currentVersion
    Init->>Init: in-sync: no notice
  else Stamp differs from current
    File-->>ReadStamp: version string
    ReadStamp-->>Init: version differs from currentVersion
    Init->>Notice: Current: stamp, Target: currentVersion
  end

Loading

sequenceDiagram
  participant updateRun as updateRun
  participant runSkillsAndStamp as runSkillsAndStamp
  participant ReadStamp as ReadStamp()
  participant RunSkillsUpdate as Updater.RunSkillsUpdate()
  participant WriteStamp as WriteStamp()
  
  updateRun->>updateRun: check if binary current
  alt Binary is current
    updateRun->>runSkillsAndStamp: pass updater, io, version
    runSkillsAndStamp->>ReadStamp: read persisted stamp
    alt Stamp matches target
      ReadStamp-->>runSkillsAndStamp: in_sync
      runSkillsAndStamp-->>updateRun: skip npx
    else Stamp differs or missing
      runSkillsAndStamp->>RunSkillsUpdate: invoke npm update
      RunSkillsUpdate-->>runSkillsAndStamp: NpmResult (nil or error)
      runSkillsAndStamp->>WriteStamp: persist new version
      WriteStamp-->>runSkillsAndStamp: warn only on failure
      runSkillsAndStamp-->>updateRun: sync result
    end
  else Binary needs update
    updateRun->>RunSkillsUpdate: invoke skills update path
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• larksuite/cli#965: Directly related migration of the same cmd/update/update.go and internal/skillscheck/* skills-sync plumbing from state/version logic to stamp-based logic with corresponding test fixtures.
• larksuite/cli#723: Related core feature work implementing stamp-based skills drift notice and unified runSkillsAndStamp update logic with the same _notice.skills / skills.stamp code paths.
• larksuite/cli#464: Related simplification of scripts/install-wizard.js's "install AI skills" flow from old version/state-based syncing to simpler "check then npx skills add" behavior.

Suggested labels

size/L, feature

Suggested reviewers

• liangshuo-1
• sang-neo03

Poem

🐰 A stamp replaces state so grand,
Version checks now fit in hand,
No sync storms, just dedup's art,
Lighter files from the start,
Skills persist with simpler heart!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is largely incomplete. It contains only a revert statement and placeholder text ('Change 1', 'Change 2') instead of actual changes, and test checkboxes remain unchecked without completion details.	Replace placeholder changes with actual modifications made by this revert (e.g., removal of stamp-based persistence, state.go deletion, sync.go removal). Specify how the revert was tested and mark the test plan checkboxes accordingly.
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.85% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: reverting the incremental skills sync feature. It is clear, specific, and concise.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/incremental-skills-update

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

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.}

[codecov]

Codecov Report

❌ Patch coverage is 98.03922% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.62%. Comparing base (e19e090) to head (b770a68).

Files with missing lines	Patch %	Lines
internal/selfupdate/updater.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1006      +/-   ##
==========================================
- Coverage   67.68%   67.62%   -0.07%     
==========================================
  Files         576      575       -1     
  Lines       54548    54337     -211     
==========================================
- Hits        36919    36743     -176     
+ Misses      14568    14550      -18     
+ Partials     3061     3044      -17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@b770a6853c247ed14067eac4a6483feeee96e4b2

🧩 Skill update

npx skills add larksuite/cli#feat/incremental-skills-update -y -g