From 8b788205a4c47ddf58c6d9d8a8c43a662110eb10 Mon Sep 17 00:00:00 2001 From: "niuchong.1" Date: Sun, 10 May 2026 22:09:08 +0800 Subject: [PATCH] fix: silence misleading "skills not installed" startup notice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the cold-start _notice.skills that fires whenever ~/.lark-cli/skills.stamp is missing. The stamp is written exclusively by `lark-cli update`, so users who installed skills via `npx skills add larksuite/cli -g` (the documented path) saw the notice on every run despite a fully populated ~/.agents/skills/. The version-drift notice (stamp != binary) is preserved unchanged for users who opted into tracking by running `lark-cli update`. - internal/skillscheck/check.go: Init returns silently on empty stamp - internal/skillscheck/notice.go: drop dead cold-start branch in Message; Current field is now guaranteed non-empty - tests updated in skillscheck package + cmd/root_integration_test.go to assert the new contract No new files, no env vars, no JSON schema change. The _notice.skills shape stays {current, target, message} — only the cold-start message string is no longer possible. --- cmd/root_integration_test.go | 23 +++++++++-------------- internal/skillscheck/check.go | 22 ++++++++++++++++------ internal/skillscheck/check_test.go | 10 +++------- internal/skillscheck/notice.go | 14 +++++++------- internal/skillscheck/notice_test.go | 5 ----- 5 files changed, 35 insertions(+), 39 deletions(-) diff --git a/cmd/root_integration_test.go b/cmd/root_integration_test.go index 98173869b5..50271b323b 100644 --- a/cmd/root_integration_test.go +++ b/cmd/root_integration_test.go @@ -504,10 +504,12 @@ func TestIntegration_Shortcut_BusinessError_OutputsEnvelope(t *testing.T) { }) } -// TestSetupNotices_ColdStart verifies that when no skills stamp exists, -// the composed PendingNotice provider includes a "skills" key with an -// empty Current and the cold-start message. -func TestSetupNotices_ColdStart(t *testing.T) { +// TestSetupNotices_ColdStart_NoNotice verifies that a missing stamp +// produces no skills key in the composed notice. Users who installed +// skills via `npx skills add` (no stamp) must not see the misleading +// "not installed" notice — only `lark-cli update` users opt into the +// drift tracker. +func TestSetupNotices_ColdStart_NoNotice(t *testing.T) { clearNoticeEnv(t) dir := t.TempDir() t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) @@ -530,17 +532,10 @@ func TestSetupNotices_ColdStart(t *testing.T) { notice := output.GetNotice() if notice == nil { - t.Fatal("GetNotice() = nil, want non-nil for cold start") + return // expected — no pending notices at all } - skills, ok := notice["skills"].(map[string]interface{}) - if !ok { - t.Fatalf("notice.skills missing, got %+v", notice) - } - if skills["current"] != "" || skills["target"] != "1.0.21" { - t.Errorf("notice.skills = %+v, want {current:\"\", target:\"1.0.21\"}", skills) - } - if msg, _ := skills["message"].(string); msg != "lark-cli skills not installed, run: lark-cli update" { - t.Errorf("notice.skills.message = %q, want cold-start message", msg) + if _, ok := notice["skills"]; ok { + t.Errorf("notice.skills present in cold-start state, want absent: %+v", notice) } } diff --git a/internal/skillscheck/check.go b/internal/skillscheck/check.go index c88a89450a..429117a18f 100644 --- a/internal/skillscheck/check.go +++ b/internal/skillscheck/check.go @@ -4,9 +4,9 @@ package skillscheck // Init runs the synchronous skills version check. Stores a StaleNotice -// when the local stamp does not match currentVersion. Safe to call -// from cmd/root.go before rootCmd.Execute(); zero network, zero -// subprocess — only a local stamp file read. +// when the local stamp records a version that does not match +// currentVersion. Safe to call from cmd/root.go before rootCmd.Execute(); +// zero network, zero subprocess — only a local stamp file read. // // Skip rules: see shouldSkip (CI envs, DEV builds, non-release semver, // LARKSUITE_CLI_NO_SKILLS_NOTIFIER opt-out). @@ -15,10 +15,12 @@ package skillscheck // - shouldSkip rule met // - ReadStamp returns an I/O error other than ENOENT // - Stamp matches currentVersion (in-sync) +// - Stamp is missing (cold start) — only users who ran `lark-cli update` +// opt into drift tracking; npx-only installs are intentionally silent. func Init(currentVersion string) { // Clear any stale notice from a prior call so early returns below - // (skip rules / read errors / in-sync) leave pending == nil instead - // of preserving a stale value from a previous Init invocation. + // (skip rules / read errors / cold start / in-sync) leave pending == nil + // instead of preserving a stale value from a previous Init invocation. SetPending(nil) if shouldSkip(currentVersion) { return @@ -28,11 +30,19 @@ func Init(currentVersion string) { // Fail closed — don't nag for a transient FS problem. return } + if stamp == "" { + // Cold start: the stamp is written exclusively by `lark-cli update` + // (runSkillsAndStamp). Users who installed skills via + // `npx skills add larksuite/cli -g` have no stamp yet — they must + // not be nagged with "skills not installed", since the on-disk + // skills directory may already be fully populated. + return + } if stamp == currentVersion { return } SetPending(&StaleNotice{ - Current: stamp, // "" when never synced + Current: stamp, // guaranteed non-empty under the new contract Target: currentVersion, }) } diff --git a/internal/skillscheck/check_test.go b/internal/skillscheck/check_test.go index f2443c4621..64525bc5a8 100644 --- a/internal/skillscheck/check_test.go +++ b/internal/skillscheck/check_test.go @@ -29,17 +29,13 @@ func TestInit_InSync_NoNotice(t *testing.T) { } } -func TestInit_ColdStart_NoticeWithEmptyCurrent(t *testing.T) { +func TestInit_ColdStart_NoNotice(t *testing.T) { clearSkillsSkipEnv(t) resetPending(t) t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) Init("1.0.21") - got := GetPending() - if got == nil { - t.Fatal("GetPending() = nil, want non-nil for cold start") - } - if got.Current != "" || got.Target != "1.0.21" { - t.Errorf("notice = %+v, want {Current:\"\", Target:\"1.0.21\"}", got) + if got := GetPending(); got != nil { + t.Errorf("GetPending() = %+v, want nil (cold start is silent)", got) } } diff --git a/internal/skillscheck/notice.go b/internal/skillscheck/notice.go index 93b1d9dfb0..b1f972218b 100644 --- a/internal/skillscheck/notice.go +++ b/internal/skillscheck/notice.go @@ -15,20 +15,20 @@ import ( // StaleNotice signals that the locally synced skills version does not // match the running binary. Current is the last successfully synced -// version (or "" when never synced); Target is the running binary -// version. Mirrors internal/update.UpdateInfo's pending-notice pattern. +// version (always non-empty — Init no longer emits a notice on cold +// start). Target is the running binary version. Mirrors +// internal/update.UpdateInfo's pending-notice pattern. type StaleNotice struct { Current string `json:"current"` Target string `json:"target"` } // Message returns a single-line, AI-agent-parseable description of the -// gap plus the canonical fix command. Mirrors internal/update.UpdateInfo.Message -// in style ("..., run: lark-cli update" suffix). +// drift plus the canonical fix command. Mirrors internal/update.UpdateInfo.Message +// in style ("..., run: lark-cli update" suffix). Current is guaranteed +// non-empty because Init only emits a StaleNotice for the drift case +// (stamp present and != binary version). func (s *StaleNotice) Message() string { - if s.Current == "" { - return "lark-cli skills not installed, run: lark-cli update" - } return fmt.Sprintf( "lark-cli skills %s out of sync with binary %s, run: lark-cli update", s.Current, s.Target, diff --git a/internal/skillscheck/notice_test.go b/internal/skillscheck/notice_test.go index 688ed57c14..575ab7bd69 100644 --- a/internal/skillscheck/notice_test.go +++ b/internal/skillscheck/notice_test.go @@ -14,11 +14,6 @@ func TestStaleNotice_Message(t *testing.T) { n StaleNotice want string }{ - { - "cold_start", - StaleNotice{Current: "", Target: "1.0.21"}, - "lark-cli skills not installed, run: lark-cli update", - }, { "drift", StaleNotice{Current: "1.0.20", Target: "1.0.21"},