fix(skills): delete a skill only when the user asks for it (#536)

The tool described delete as "delete skills you created", which the code never
enforced and which does not fit how skills are made: most are written by the
user and the agent together, so who made the first call says nothing about
whether anyone minds the skill going. With the post-turn review gone there is
no case left for the agent deleting unasked, so the rule is simply: delete an
attached skill only when the user asks, whoever wrote it. Stated in the tool
description, the delete schema, and the manage-skills guidance (1.3; 1.2's
fingerprint recorded). No code gate: the request lives in the conversation, so
only the model can attest it either way.

Co-authored-by: Claude <noreply@anthropic.com>

Author: Leo310

AGENTS.md

@@ -129,7 +129,7 @@ August 2026; recover it from git history if useful, but do not trust it.)
   - `Agents/Skills/` — skill `<name>/SKILL.md` dirs. **Everything is a skill**: the 4 former capabilities (`vault`, `notes`, `web`, `update`) ship as bundled **core skills** whose body is their guidance and whose `allowed-tools` frontmatter attaches built-in tools; user skills are the same thing with no attached tools. Discovery scans this folder and treats every `SKILL.md` dir as a skill (no reserved names, no `GUIDANCE.md`).
   - `Agents/<Agent Name>/AGENT.md` — one folder per agent, holding its definition note. The note's **body IS the system prompt**: base instructions, the `# Current Date` section, and the `# Memory` section, all in one editable place. There is no memory toggle — an agent participates in memory iff its note still has that section, which the user can simply delete. The folder is the unit rename/duplicate/delete operate on (see `PromptFilesService`).
   The whole tree is plugin machinery, excluded from indexing/search/graph via `isAgentFilePath` in `utils/fileFiltering.ts` (path helpers in `utils/agentPaths.ts`). One deliberate exemption: `list_directory` always shows `Agents/Memories/` — memory notes are absent from the search index, so that listing is the agent's only memory-discovery path (the default `# Memory` section directs it there). Legacy installs are consolidated on first run (`SkillsService.migrateAgentFolder`, from the old top-level `Skills/` folder or the pre-vault `<configDir>/skills`); the v6 `migrateCoreSkills` deletes orphaned per-core-skill `GUIDANCE.md` files so the bundled core-skill `SKILL.md`s seed cleanly into the same dirs.
-- `src/skills/` — Two-phase skill system (Agent Skills style): discover frontmatter on startup, load full content on demand. Bundled defaults under `src/skills/defaults/` (including the `vault`/`notes`/`web`/`manage-skills` core skills — the last one was named `update-skills` before it gained create/delete, migrated in schema v8). **`allowed-tools` is load-bearing**: `AgentManager.buildToolsForAgent` binds a built-in tool only when some *enabled* skill attaches it via `allowed-tools` AND the per-tool `toolsConfig[toolId].enabled` override hasn't vetoed it. All skill guidance reaches the model lazily — advertised by description in the `# Skills` `<available_skills>` block, body loaded on demand via `load_skill`; a skill whose declared built-in tools are *all* vetoed by per-tool overrides is hidden from both surfaces (`AgentManager.skillHasUsableTools`), so the model is never taught a tool that won't be bound. The `manage_skills` tool (the "Manage Skills" core skill, on by default since schema v14; the Tools modal turns it off) lets an agent create new skills, revise its own attached skills, or delete skills it created; unlike `manage_notes`, every operation applies immediately with no `pendingChangesStore` review step — creating a skill is the same action as attaching it. A created skill gets no explicit `agent.skills` entry, so it reads as attached (`?? true`) the moment its file exists on disk. `allowedTools` on a new skill is filtered through a fixed read-only allow-list, never passed through verbatim — the one guard against an agent granting itself new capability via a skill it just wrote. Deleting a built-in core skill is refused (it would just reappear via `bootstrapDefaultSkills` on next startup). Because a tool can be attached by more than one skill (e.g. an integration skill re-declaring a tool a core skill already owns), per-tool overrides are **not** configured per-skill — they live in one agent-level `ToolsModal` (opened from the "Tools" row in the Agent editor's General section), which lists all `BUILT_IN_TOOL_IDS` flat and shows each tool's attaching skill(s).
+- `src/skills/` — Two-phase skill system (Agent Skills style): discover frontmatter on startup, load full content on demand. Bundled defaults under `src/skills/defaults/` (including the `vault`/`notes`/`web`/`manage-skills` core skills — the last one was named `update-skills` before it gained create/delete, migrated in schema v8). **`allowed-tools` is load-bearing**: `AgentManager.buildToolsForAgent` binds a built-in tool only when some *enabled* skill attaches it via `allowed-tools` AND the per-tool `toolsConfig[toolId].enabled` override hasn't vetoed it. All skill guidance reaches the model lazily — advertised by description in the `# Skills` `<available_skills>` block, body loaded on demand via `load_skill`; a skill whose declared built-in tools are *all* vetoed by per-tool overrides is hidden from both surfaces (`AgentManager.skillHasUsableTools`), so the model is never taught a tool that won't be bound. The `manage_skills` tool (the "Manage Skills" core skill, on by default since schema v14; the Tools modal turns it off) lets an agent create new skills, revise its own attached skills, or delete a skill the user asked it to delete (whoever wrote it; the rule is guidance, since only the conversation holds the request); unlike `manage_notes`, every operation applies immediately with no `pendingChangesStore` review step — creating a skill is the same action as attaching it. A created skill gets no explicit `agent.skills` entry, so it reads as attached (`?? true`) the moment its file exists on disk. `allowedTools` on a new skill is filtered through a fixed read-only allow-list, never passed through verbatim — the one guard against an agent granting itself new capability via a skill it just wrote. Deleting a built-in core skill is refused (it would just reappear via `bootstrapDefaultSkills` on next startup). Because a tool can be attached by more than one skill (e.g. an integration skill re-declaring a tool a core skill already owns), per-tool overrides are **not** configured per-skill — they live in one agent-level `ToolsModal` (opened from the "Tools" row in the Agent editor's General section), which lists all `BUILT_IN_TOOL_IDS` flat and shows each tool's attaching skill(s).
 - `src/agent/promptFiles.ts` — File-backed store for each agent's `<Agent Name>/AGENT.md`. The code constant `DEFAULT_AGENT_PROMPT` remains the factory default the diff/reset UI compares against; the file is the editable copy. Values that must stay live are written into the body as placeholders (`{{memoryFolder}}`, `{{date}}`) and substituted by `substitutePromptPlaceholders` at assembly, so nothing stale is ever baked into stored text; assembly appends only what is irreducibly dynamic (the `# Skills` block, the no-write-tools guard). Each note carries a small plugin-managed frontmatter block (`author`, `version` — flat keys, so Obsidian's Properties UI renders them) whose `version` records the shipped baseline the body was written from, mirroring how skills version their SKILL.md; it is what makes "the default moved under YOUR edit" detectable, and it travels with the note through sync/copy (duplicating an agent copies the notes verbatim, provenance included). Everything model-facing uses the frontmatter-stripped **body** only — assembly, the diff modal, and the shipped-history fingerprints — so restamping never reads as a customization. Content is cached in memory as parsed body+version (populated at init + on vault change) so `assembleSystemPrompt` and the reactive stale-guidance getter read it without hitting disk. Editing happens in the vault note (pencil/"open note"); the `SystemPromptModal` diff modal stays for comparing against the default and resetting. Because the note lives in a folder of its own, rename/duplicate/delete are directory operations. (Skill/tool guidance is no longer stored here — it's the skill body, edited via the note / `manage_skills`.)
 - `src/stores/` — Reactive state via Svelte 5 runes (`*.svelte.ts`). `dataStore` is canonical config + secrets indirection (its non-store residents live beside it: `agentDefaults.ts` for the default agent + load-time normalization, `dataMigrations.ts` for the schema version + migration table, `staleGuidance.ts` for the prompt-staleness detector); `chatStore` owns `ChatSession` and the `SessionRegistry`, while everything that *derives* the timeline from checkpoint history (checkpoint graph, `MessagePair`s, tool timelines, summarization markers, context blocks) is the pure, rune-free `chatTimeline.ts`; `pendingChangesStore` stages note mutations for review. `state.svelte.ts` is a leaf (plugin handle only) — nothing under `stores/` may be imported by `providers/*` or `utils/*`; see the import-cycle rule below.
 - `src/components/` — Feature-vertical Svelte UI: `chat/`, `graph/`, `settings/`, `modal/`, plus shared `ui/` primitives. Markdown rendering goes through Obsidian's renderer (not a custom one). A modal whose body is a Svelte component extends `modal/SvelteModal.ts` (`mountComponent(component, props, layout?)`; it owns mount/unmount and the layout override) rather than re-implementing the lifecycle.

src/agent/tools/builtInToolDefaults.ts

@@ -185,7 +185,7 @@ export const BUILT_IN_TOOL_DEFAULTS: Record<BuiltInToolId, BuiltInToolDefault> =
 	manage_skills: {
 		displayName: "Manage Skills",
 		summary:
-			"Create new skills, revise the agent's own attached skills, or delete skills it created. Changes apply immediately. A skill's name and plugin link are locked once created.",
+			"Create new skills, revise the agent's own attached skills, or delete a skill when you ask it to. Changes apply immediately. A skill's name and plugin link are locked once created.",
 		config: {
 			// On by default: the routing doctrine in the memory section and the `# Skills`
 			// header both send task lessons into the skill that was used, and the post-turn
@@ -194,7 +194,7 @@ export const BUILT_IN_TOOL_DEFAULTS: Record<BuiltInToolId, BuiltInToolDefault> =
 			enabled: true,
 			name: "manage_skills",
 			description:
-				"Create new skills, revise your own attached skills, or delete skills you created. Changes apply immediately. A skill's name and plugin link are locked once created; only the body and description can change.",
+				"Create new skills, revise your own attached skills, or delete a skill when the user asks you to. Changes apply immediately. A skill's name and plugin link are locked once created; only the body and description can change.",
 		},
 	},
 };

src/agent/tools/manageSkills.ts

@@ -166,7 +166,11 @@ const createOperationSchema = z.object({
 
 const deleteOperationSchema = z.object({
 	type: z.literal("delete"),
-	name: z.string().describe("The name of the skill to delete. Built-in core skills cannot be deleted."),
+	name: z
+		.string()
+		.describe(
+			"The name of the skill to delete. Only when the user asked for this skill to be deleted. Built-in core skills cannot be deleted.",
+		),
 });
 
 const patchOperationSchema = z.object({
@@ -243,8 +247,8 @@ function rejectInvalidRevision(
 type ManageSkillsInput = z.infer<typeof manageSkillsSchema>;
 
 /**
- * Tool letting an agent create new skills, revise skills attached to it, or delete skills it
- * created. All three operations apply immediately — there is no staging/review step, unlike
+ * Tool letting an agent create new skills, revise skills attached to it, or delete an attached
+ * skill the user asked it to delete. All three operations apply immediately — there is no staging/review step, unlike
  * manage_notes. A created skill is given no explicit `agent.skills` entry, so it reads as
  * attached the moment its file exists (agent.skills[id]?.enabled ?? true): creating IS attaching,
  * with no separate manual "enable" step.
@@ -412,7 +416,7 @@ export function createManageSkillsTool(skillsService: SkillsService | undefined,
 		},
 		{
 			name: "manage_skills",
-			description: `Create new skills, revise your own attached skills, or delete skills you created. Changes apply immediately — there is no review step. To revise, load the skill with load_skill first, then patch the exact passage that needs changing (update replaces the whole body; use it only to restructure). A skill's name and plugin link are locked once created. Attached skills: ${attachedAtBuild.join(", ")}`,
+			description: `Create new skills, revise your own attached skills, or delete a skill when the user asks you to. Changes apply immediately — there is no review step. To revise, load the skill with load_skill first, then patch the exact passage that needs changing (update replaces the whole body; use it only to restructure). A skill's name and plugin link are locked once created. Attached skills: ${attachedAtBuild.join(", ")}`,
 			schema,
 		},
 	);

src/skills/defaults/manage-skills/SKILL.md

@@ -1,10 +1,10 @@
 ---
 name: manage-skills
-description: Create, revise, or delete skills with manage_skills — author new skills, fold verified knowledge into an existing skill's instructions, or remove a skill you created. Changes apply immediately. Load this before editing or creating a skill.
+description: Create, revise, or delete skills with manage_skills — author new skills, fold verified knowledge into an existing skill's instructions, or remove a skill the user asked to delete. Changes apply immediately. Load this before editing or creating a skill.
 allowed-tools: manage_skills
 metadata:
   author: "S2B"
-  version: "1.2"
+  version: "1.3"
   category: "core"
 ---
 
@@ -48,5 +48,6 @@ an allowed subset are granted, others are silently dropped.
 - Keep new skills narrow and instructions concrete — write down only what you'd actually want to
   remember doing again.
 
-**Delete** — remove a skill you created, immediately and without confirmation. Built-in core
-skills cannot be deleted.
+**Delete** — remove a skill only when the user asks for that skill to be deleted, whoever
+wrote it. Never delete one on your own initiative, to tidy up or because it looks unused. It
+applies immediately, with no confirmation step. Built-in core skills cannot be deleted.

src/skills/shippedSkills.ts

@@ -91,6 +91,8 @@ const PRIOR_SKILL_FINGERPRINTS: ReadonlyMap<string, ReadonlyMap<string, string>>
 		new Map([
 			["1.0", "4f7b8ff2b47b60e2"],
 			["1.1", "6243bd2c4cf42f9a"],
+			// 1.2 (2.3.0 betas): before the rule that a skill is deleted only when the user asks.
+			["1.2", "7b0e742eaeffaa0a"],
 		]),
 	],
 	// 1.0 (shipped in 2.2.0): before the note that memory-folder writes apply immediately.