Stop reporting skill-update no-op cycles as errors

packages/workspace-server/src/services/posthog-plugin/posthog-plugin.test.ts

@@ -433,8 +433,9 @@ describe("PosthogPluginService", () => {
       await expect(service.updateSkills()).resolves.toBeUndefined();
     });
 
-    it("handles missing skills dir in archive", async () => {
-      // Extraction creates no skills directory
+    it("reports a clear, actionable error when nothing downloads and no cache exists", async () => {
+      // Extraction creates no skills directory and there is no pre-existing
+      // skills cache to fall back on — the genuine failure case.
       mockExtractZip.mockImplementation(
         async (_zipPath: string, extractDir: string) => {
           vol.mkdirSync(`${extractDir}/random-dir`, { recursive: true });
@@ -447,6 +448,42 @@ describe("PosthogPluginService", () => {
       await service.updateSkills();
 
       expect(handler).not.toHaveBeenCalled();
+      // The reported error must be the clear, user-useful message, not the
+      // opaque "No skills found from any source".
+      expect(mockAnalytics.captureException).toHaveBeenCalledTimes(1);
+      const reportedError = mockAnalytics.captureException.mock
+        .calls[0][0] as Error;
+      expect(reportedError.message).not.toContain("No skills found");
+      expect(reportedError.message).toContain("Couldn't download skills");
+      expect(reportedError.message).toContain("retry automatically");
+    });
+
+    it("keeps existing skills and stays silent when a download cycle is empty", async () => {
+      // Simulate a previously-downloaded skills cache from an earlier run.
+      vol.mkdirSync(`${RUNTIME_SKILLS_DIR}/cached-skill`, { recursive: true });
+      vol.writeFileSync(
+        `${RUNTIME_SKILLS_DIR}/cached-skill/SKILL.md`,
+        "# Cached",
+      );
+
+      // Both downloads fail this cycle (e.g. transient network failure).
+      mockFetch.mockRejectedValue(new Error("Network error"));
+
+      const handler = vi.fn();
+      service.on("skillsUpdated", handler);
+      await service.updateSkills();
+
+      // Existing cache is preserved, no false-alarm exception, no update event,
+      // and the staging dir is cleaned up.
+      expect(
+        vol.readFileSync(
+          `${RUNTIME_SKILLS_DIR}/cached-skill/SKILL.md`,
+          "utf-8",
+        ),
+      ).toBe("# Cached");
+      expect(mockAnalytics.captureException).not.toHaveBeenCalled();
+      expect(handler).not.toHaveBeenCalled();
+      expect(vol.existsSync(`${RUNTIME_SKILLS_DIR}.new`)).toBe(false);
     });
 
     it("cleans up temp dir even on error", async () => {

packages/workspace-server/src/services/posthog-plugin/posthog-plugin.ts

@@ -183,7 +183,11 @@ export class PosthogPluginService extends TypedEventEmitter<PosthogPluginEvents>
       });
 
       if (result.success) {
-        this.emit("skillsUpdated", true);
+        // Only signal listeners when the cache actually changed; an empty
+        // download cycle succeeds as a no-op (result.data.updated === false).
+        if (result.data.updated) {
+          this.emit("skillsUpdated", true);
+        }
       } else {
         this.log.warn("Skills update failed", {
           error: result.error,

packages/workspace-server/src/services/posthog-plugin/update-skills-saga.ts

@@ -108,13 +108,54 @@ export class UpdateSkillsSaga extends Saga<
       }
     });
 
-    // Step 3: validate skills (fatal if empty → triggers rollback of step 1)
-    await this.readOnlyStep("validate-skills", async () => {
-      const entries = await readdir(newSkillsDir);
-      if (entries.length === 0) {
-        throw new Error("No skills found from any source");
+    // Step 3: validate skills. An empty staging dir means both downloads
+    // produced nothing this cycle (e.g. a transient network failure — the
+    // download steps above are intentionally non-fatal). The existing skills
+    // cache and the bundled skills remain in place, so this is a no-op cycle,
+    // not a failure: skip the swap and try again on the next interval rather
+    // than throwing, which would surface a misleading "no skills" exception.
+    const stagedSkillCount = await this.readOnlyStep(
+      "validate-skills",
+      async () => {
+        const entries = await readdir(newSkillsDir);
+        return entries.length;
+      },
+    );
+
+    if (stagedSkillCount === 0) {
+      // Both downloads produced nothing this cycle. Clean up the empty
+      // staging dir, then decide whether this is a recoverable no-op or a
+      // genuine failure based on whether we have skills to fall back on.
+      await rm(newSkillsDir, { recursive: true, force: true });
+
+      const hasCachedSkills =
+        existsSync(input.runtimeSkillsDir) &&
+        (await readdir(input.runtimeSkillsDir)).length > 0;
+
+      if (hasCachedSkills) {
+        // A transient blip (e.g. network failure — the download steps above
+        // are non-fatal) shouldn't surface as an error: the previously
+        // downloaded skills are still in place. Skip the swap and retry next
+        // interval.
+        this.log.warn(
+          "No skills downloaded this cycle; keeping existing skills cache",
+        );
+        return { updated: false };
       }
-    });
+
+      // Nothing downloaded and no cache to fall back on. Surface a clear,
+      // actionable message rather than the opaque "No skills found from any
+      // source" — it tells the user what happened, that they're not stuck
+      // (bundled skills still work), and that it will recover on its own.
+      throw new Error(
+        "Couldn't download skills from PostHog (the skills and context-mill " +
+          "sources both returned nothing, likely a temporary network or " +
+          "server issue) and no previously downloaded skills were cached. " +
+          "The skills bundled with this version of PostHog Code are still " +
+          "available, and PostHog Code will retry automatically on the next " +
+          "update.",
+      );
+    }
 
     // Step 4: atomic swap
     const oldSkillsDir = `${input.runtimeSkillsDir}.old`;