diff --git a/apps/memos-local-plugin/core/skill/crystallize.ts b/apps/memos-local-plugin/core/skill/crystallize.ts index c45c5453d..17e9ad146 100644 --- a/apps/memos-local-plugin/core/skill/crystallize.ts +++ b/apps/memos-local-plugin/core/skill/crystallize.ts @@ -470,11 +470,34 @@ function capString(s: string, cap: number): string { /** * A sensible default validator used both in production and in tests. - * Throws if the draft is structurally unusable (no name, no steps, no summary). + * Throws only when the draft is structurally unusable (no name). Missing + * summary/steps are repaired from the remaining fields instead — LLMs (e.g. + * deepseek-v4-flash) routinely return valid JSON drafts that omit `summary` + * or the `steps` array, and rejecting those stalls the crystallizer queue + * (see issue #2143). */ export function defaultDraftValidator(draft: SkillCrystallizationDraft): void { if (!draft.name) throw new Error("skill.crystallize.invalid: missing name"); - if (!draft.summary) throw new Error("skill.crystallize.invalid: missing summary"); - if (draft.steps.length === 0) - throw new Error("skill.crystallize.invalid: missing steps"); + if (!draft.summary) { + // Auto-generate a summary from the richest available field. Use `||` not + // `??`: LLM JSON emits empty strings, and `??` only falls through on + // null/undefined. + const autoSummary = + draft.steps?.[0]?.body || + draft.steps?.[0]?.title || + draft.displayTitle || + draft.name || + "skill procedure"; + draft.summary = autoSummary.slice(0, 200); + } + if (!draft.steps || draft.steps.length === 0) { + // Auto-generate a single step when the LLM omits the steps array + // (mirror of the summary fallback chain above). + const autoBody = draft.summary || draft.displayTitle || draft.name || ""; + if (autoBody) { + draft.steps = [{ title: "Execute the fix", body: autoBody.slice(0, 2000) }]; + } else { + throw new Error("skill.crystallize.invalid: missing steps"); + } + } } diff --git a/apps/memos-local-plugin/tests/unit/skill/crystallize-validator.test.ts b/apps/memos-local-plugin/tests/unit/skill/crystallize-validator.test.ts new file mode 100644 index 000000000..348a2ac72 --- /dev/null +++ b/apps/memos-local-plugin/tests/unit/skill/crystallize-validator.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from "vitest"; + +import { defaultDraftValidator } from "../../../core/skill/crystallize.js"; +import { makeDraft } from "./_helpers.js"; + +describe("defaultDraftValidator", () => { + it("passes a complete draft through unchanged", () => { + const draft = makeDraft(); + expect(() => defaultDraftValidator(draft)).not.toThrow(); + expect(draft.summary).toBe("Ensure system libs exist before pip install on alpine."); + expect(draft.steps).toHaveLength(3); + }); + + it("never throws for a missing summary (issue #2143)", () => { + const draft = makeDraft({ summary: "" }); + expect(() => defaultDraftValidator(draft)).not.toThrow(); + }); + + it("auto-generates summary from the first step body when omitted", () => { + const draft = makeDraft({ summary: "" }); + defaultDraftValidator(draft); + expect(draft.summary).toBe("inspect the pip error for missing .so names"); + }); + + it("falls back through step title, displayTitle, then name for the summary", () => { + const draft = makeDraft({ summary: "", steps: [] }); + defaultDraftValidator(draft); + expect(draft.summary).toBe("Alpine pip install with system deps"); + }); + + it("caps the auto-generated summary at 200 chars", () => { + const longBody = "x".repeat(500); + const draft = makeDraft({ + summary: "", + steps: [{ title: "t", body: longBody }], + }); + defaultDraftValidator(draft); + expect(draft.summary).toBe("x".repeat(200)); + }); + + it("uses || not ?? — an empty-string summary still triggers the fallback", () => { + const draft = makeDraft({ summary: "", steps: [] }); + defaultDraftValidator(draft); + expect(draft.summary).not.toBe(""); + }); + + it("auto-generates a single step when the steps array is empty", () => { + const draft = makeDraft({ steps: [] }); + defaultDraftValidator(draft); + expect(draft.steps).toEqual([ + { + title: "Execute the fix", + body: "Ensure system libs exist before pip install on alpine.", + }, + ]); + }); + + it("still rejects a draft with no name", () => { + const draft = makeDraft({ name: "" }); + expect(() => defaultDraftValidator(draft)).toThrow(/missing name/); + }); +}); diff --git a/apps/memos-local-plugin/tests/unit/skill/crystallize.test.ts b/apps/memos-local-plugin/tests/unit/skill/crystallize.test.ts index a2e0e63eb..bcaccaf67 100644 --- a/apps/memos-local-plugin/tests/unit/skill/crystallize.test.ts +++ b/apps/memos-local-plugin/tests/unit/skill/crystallize.test.ts @@ -230,7 +230,10 @@ describe("skill/crystallize", () => { expect(r.modelRefusal?.content).toContain("I cannot process this request"); }); - it("rejects drafts that the validator flags as invalid", async () => { + it("repairs drafts the strict validator would have rejected (issue #2143)", async () => { + // A draft with an empty summary AND no steps used to be rejected with + // skill.crystallize.invalid: missing summary / missing steps. The lenient + // validator auto-generates both, so the same draft now crystallizes. const llm = fakeLlm({ completeJson: { "skill.crystallize": makeDraft({ steps: [], summary: "" }) as unknown, @@ -240,6 +243,10 @@ describe("skill/crystallize", () => { { policy: mkPolicy(), evidence: [mkTrace("tr_1", "x")], namingSpace: [] }, { llm, log, config: makeSkillConfig(), validate: defaultDraftValidator }, ); - expect(r.ok).toBe(false); + expect(r.ok).toBe(true); + if (r.ok) { + expect(r.draft.summary).not.toBe(""); + expect(r.draft.steps.length).toBeGreaterThan(0); + } }); });