From bbafa748fb25db7b61be03e13e0bfb102a08a6b9 Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 7 May 2026 10:14:06 -0500 Subject: [PATCH] Address LLM Wiki greptile feedback --- .../plugins/plugin-llm-wiki/src/manifest.ts | 6 +- .../plugins/plugin-llm-wiki/src/ui/index.tsx | 7 +- packages/plugins/plugin-llm-wiki/src/wiki.ts | 2 + .../plugin-llm-wiki/tests/plugin.spec.ts | 29 +++++++- packages/shared/src/types/plugin.ts | 3 + .../__tests__/plugin-managed-skills.test.ts | 21 ++++++ server/src/services/plugin-managed-skills.ts | 71 +++++++++++++++++-- 7 files changed, 127 insertions(+), 12 deletions(-) diff --git a/packages/plugins/plugin-llm-wiki/src/manifest.ts b/packages/plugins/plugin-llm-wiki/src/manifest.ts index f1f7c5a1bb8..2fe0ad823d7 100644 --- a/packages/plugins/plugin-llm-wiki/src/manifest.ts +++ b/packages/plugins/plugin-llm-wiki/src/manifest.ts @@ -161,9 +161,9 @@ const manifest: PaperclipPluginManifestV1 = { adapterType: "claude_local", adapterPreference: ["claude_local", "codex_local", "gemini_local", "opencode_local", "cursor", "pi_local"], adapterConfig: { - dangerouslySkipPermissions: true, - dangerouslyBypassApprovalsAndSandbox: true, - sandbox: false, + dangerouslySkipPermissions: false, + dangerouslyBypassApprovalsAndSandbox: false, + sandbox: true, paperclipSkillSync: { desiredSkills: WIKI_MANAGED_SKILL_CANONICAL_KEYS } diff --git a/packages/plugins/plugin-llm-wiki/src/ui/index.tsx b/packages/plugins/plugin-llm-wiki/src/ui/index.tsx index a1db8223c98..e73704e3e66 100644 --- a/packages/plugins/plugin-llm-wiki/src/ui/index.tsx +++ b/packages/plugins/plugin-llm-wiki/src/ui/index.tsx @@ -159,6 +159,7 @@ type ManagedSkill = { status: string; skillId?: string | null; resourceKey?: string | null; + defaultDrift?: { changedFiles: string[] } | null; skill?: { id?: string; name?: string; @@ -4878,9 +4879,11 @@ function buildSkillHealthItems(skills: ManagedSkill[]): RoutineHealthItem[] { } return skills.map((skill) => ({ label: skillLabel(skill), - ok: managedSkillIsReady(skill), + ok: managedSkillIsReady(skill) && !skill.defaultDrift?.changedFiles.length, detail: managedSkillIsReady(skill) - ? `${skillLabel(skill)} is installed in the company skill library.` + ? skill.defaultDrift?.changedFiles.length + ? `${skillLabel(skill)} differs from the plugin default: ${skill.defaultDrift.changedFiles.join(", ")}.` + : `${skillLabel(skill)} is installed in the company skill library.` : `${skillLabel(skill)} is not installed in the company skill library.`, })); } diff --git a/packages/plugins/plugin-llm-wiki/src/wiki.ts b/packages/plugins/plugin-llm-wiki/src/wiki.ts index 5a5d0d3c666..9621d1874f7 100644 --- a/packages/plugins/plugin-llm-wiki/src/wiki.ts +++ b/packages/plugins/plugin-llm-wiki/src/wiki.ts @@ -183,6 +183,7 @@ export type WikiSkillResource = { resourceKey: string; skill: PluginManagedSkillResolution["skill"]; details: { name: string; key: string; description: string | null } | null; + defaultDrift?: { changedFiles: string[] } | null; }; export type WikiResourceOption = { @@ -1591,6 +1592,7 @@ function skillResource(resolved: PluginManagedSkillResolution): WikiSkillResourc resourceKey: resolved.resourceKey, skill: resolved.skill, details: skillDetails(resolved.skill), + defaultDrift: resolved.defaultDrift ?? null, }; } diff --git a/packages/plugins/plugin-llm-wiki/tests/plugin.spec.ts b/packages/plugins/plugin-llm-wiki/tests/plugin.spec.ts index c8801c36617..5513f52ad90 100644 --- a/packages/plugins/plugin-llm-wiki/tests/plugin.spec.ts +++ b/packages/plugins/plugin-llm-wiki/tests/plugin.spec.ts @@ -668,9 +668,9 @@ describe("LLM Wiki plugin scaffold", () => { expect(manifest.agents?.[0]?.agentKey).toBe("wiki-maintainer"); expect(manifest.agents?.[0]?.adapterType).toBe("claude_local"); expect(manifest.agents?.[0]?.adapterConfig).toMatchObject({ - dangerouslySkipPermissions: true, - dangerouslyBypassApprovalsAndSandbox: true, - sandbox: false, + dangerouslySkipPermissions: false, + dangerouslyBypassApprovalsAndSandbox: false, + sandbox: true, }); expect(manifest.agents?.[0]?.instructions?.entryFile).toBe("AGENTS.md"); expect(manifest.agents?.[0]?.instructions?.content).toContain("You are the maintainer of this personal wiki"); @@ -1137,6 +1137,29 @@ Duplicate headings receive stable suffixes. expect(markup).toContain("Re-sync skills"); }); + it("shows managed skill default drift in setup health", () => { + mockPathname = "/PAP/wiki/settings"; + mockSettingsManagedSkills = [{ + status: "resolved", + skillId: "skill-1", + resourceKey: WIKI_MAINTAINER_SKILL_KEY, + defaultDrift: { changedFiles: ["SKILL.md"] }, + details: { + name: "LLM Wiki Maintainer", + key: WIKI_MAINTAINER_SKILL_CANONICAL_KEY, + description: null, + }, + }]; + + const markup = renderToStaticMarkup(createElement(WikiPage, { + context: { companyId: COMPANY_ID, companyPrefix: "PAP" }, + } as never)); + + expect(markup).toContain("skill issue(s) need attention"); + expect(markup).toContain("LLM Wiki Maintainer differs from the plugin default: SKILL.md."); + expect(markup).toContain("Re-sync skills"); + }); + it("renders host settings directly as the Setup page without an extra plugin heading", () => { const markup = renderToStaticMarkup(createElement(SettingsPage, { context: { companyId: COMPANY_ID, companyPrefix: "PAP" }, diff --git a/packages/shared/src/types/plugin.ts b/packages/shared/src/types/plugin.ts index 3bc776edde8..6f9629122cc 100644 --- a/packages/shared/src/types/plugin.ts +++ b/packages/shared/src/types/plugin.ts @@ -321,6 +321,9 @@ export interface PluginManagedSkillResolution { skillId: string | null; skill: CompanySkill | null; status: "missing" | "resolved" | "created" | "relinked" | "reset"; + defaultDrift?: { + changedFiles: string[]; + } | null; } /** diff --git a/server/src/__tests__/plugin-managed-skills.test.ts b/server/src/__tests__/plugin-managed-skills.test.ts index 9d5b7fe9130..de9af4d2f89 100644 --- a/server/src/__tests__/plugin-managed-skills.test.ts +++ b/server/src/__tests__/plugin-managed-skills.test.ts @@ -174,6 +174,27 @@ describeEmbeddedPostgres("plugin-managed skills", () => { expect(reset.skill?.markdown).toContain("key: \"plugin/paperclip-managed-skills-test/wiki-maintainer\""); }); + it("reports drift when installed skill files differ from plugin defaults", async () => { + const { companyId, services } = await seedCompanyAndPlugin(); + const created = await services.skills.managedReconcile({ companyId, skillKey: "wiki-maintainer" }); + expect(created.defaultDrift).toBeNull(); + + await db + .update(companySkills) + .set({ + markdown: "# Custom instructions\n", + updatedAt: new Date(), + }) + .where(eq(companySkills.id, created.skillId!)); + + const drifted = await services.skills.managedReconcile({ companyId, skillKey: "wiki-maintainer" }); + expect(drifted.status).toBe("resolved"); + expect(drifted.defaultDrift).toEqual({ changedFiles: ["SKILL.md"] }); + + const reset = await services.skills.managedReset({ companyId, skillKey: "wiki-maintainer" }); + expect(reset.defaultDrift).toBeNull(); + }); + it("adds the canonical managed key to manifest-provided markdown skills", async () => { const pluginManifest = manifest(); pluginManifest.skills = [ diff --git a/server/src/services/plugin-managed-skills.ts b/server/src/services/plugin-managed-skills.ts index 8b673b0da70..49611bbd39b 100644 --- a/server/src/services/plugin-managed-skills.ts +++ b/server/src/services/plugin-managed-skills.ts @@ -91,6 +91,20 @@ function buildPackageFiles( return files; } +function buildDeclaredSkillFiles( + pluginKey: string, + declaration: PluginManagedSkillDeclaration, +) { + const packageFiles = buildPackageFiles(pluginKey, declaration); + const root = declaration.skillKey; + const prefix = `${root}/`; + const files: Record = {}; + for (const [filePath, content] of Object.entries(packageFiles)) { + files[filePath.startsWith(prefix) ? filePath.slice(prefix.length) : filePath] = content; + } + return files; +} + function buildSkillDefaults( pluginKey: string, declaration: PluginManagedSkillDeclaration, @@ -114,6 +128,7 @@ function resolution( declaration: PluginManagedSkillDeclaration, skill: CompanySkill | null, status: PluginManagedSkillResolution["status"], + defaultDrift: PluginManagedSkillResolution["defaultDrift"] = null, ): PluginManagedSkillResolution { return { pluginKey, @@ -123,6 +138,7 @@ function resolution( skillId: skill?.id ?? null, skill, status, + defaultDrift, }; } @@ -191,6 +207,53 @@ export function pluginManagedSkillService( return skills.getById(companyId, skillId); } + async function managedSkillDefaultDrift( + companyId: string, + skill: CompanySkill | null, + declaration: PluginManagedSkillDeclaration, + ): Promise { + if (!skill) return null; + const declaredFiles = buildDeclaredSkillFiles(options.pluginKey, declaration); + const currentFiles: Record = {}; + const paths = new Set([ + ...Object.keys(declaredFiles), + ...skill.fileInventory.map((entry) => entry.path), + ]); + + for (const filePath of paths) { + if (filePath === "SKILL.md") { + currentFiles[filePath] = skill.markdown; + continue; + } + try { + currentFiles[filePath] = (await skills.readFile(companyId, skill.id, filePath))?.content ?? null; + } catch { + currentFiles[filePath] = null; + } + } + + const changedFiles = [...paths] + .filter((filePath) => (currentFiles[filePath] ?? null) !== (declaredFiles[filePath] ?? null)) + .sort((left, right) => left.localeCompare(right)); + return changedFiles.length > 0 ? { changedFiles } : null; + } + + async function resolvedSkill( + companyId: string, + declaration: PluginManagedSkillDeclaration, + skill: CompanySkill | null, + status: PluginManagedSkillResolution["status"], + ) { + return resolution( + options.pluginKey, + companyId, + declaration, + skill, + status, + await managedSkillDefaultDrift(companyId, skill, declaration), + ); + } + async function importDeclaredSkill( companyId: string, declaration: PluginManagedSkillDeclaration, @@ -224,9 +287,9 @@ export function pluginManagedSkillService( async function get(skillKey: string, companyId: string) { const declaration = declarationFor(skillKey); const binding = await getBinding(companyId, skillKey); - if (!binding) return resolution(options.pluginKey, companyId, declaration, null, "missing"); + if (!binding) return resolvedSkill(companyId, declaration, null, "missing"); const skill = await getSkill(companyId, binding.resourceId); - return resolution(options.pluginKey, companyId, declaration, skill, skill ? "resolved" : "missing"); + return resolvedSkill(companyId, declaration, skill, skill ? "resolved" : "missing"); } async function reconcile(skillKey: string, companyId: string) { @@ -250,7 +313,7 @@ export function pluginManagedSkillService( status: imported.status, }, }); - return resolution(options.pluginKey, companyId, declaration, imported.skill, imported.status); + return resolvedSkill(companyId, declaration, imported.skill, imported.status); } async function reset(skillKey: string, companyId: string) { @@ -268,7 +331,7 @@ export function pluginManagedSkillService( managedResourceKey: declaration.skillKey, }, }); - return resolution(options.pluginKey, companyId, declaration, imported.skill, "reset"); + return resolvedSkill(companyId, declaration, imported.skill, "reset"); } return {