Skip to content

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

Merged
Leo310 merged 1 commit into
mainfrom
fix/manage-skills-delete-authorship
Sep 28, 2026
Merged

Leo310 merged 1 commit into
mainfrom
fix/manage-skills-delete-authorship

Conversation

@Leo310

@Leo310 Leo310 commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

What

manage_skills described delete as "delete skills you created", but the code only checked that a skill was attached and not a core skill, so the agent could remove a skill on its own initiative.

The first version of this PR gated delete on the metadata.author: agent stamp, with a userRequested flag to override it. That was dropped after discussion: skills are mostly made 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 removed (#532), there is also no case left for the agent deleting a skill unasked.

The rule is now one sentence, whoever wrote the skill: delete an attached skill only when the user asks for it. It is stated in:

  • the tool description (the built-in default in builtInToolDefaults.ts and the tool's own),
  • the delete operation's name field,
  • the manage-skills guidance, bumped to 1.3, which also says never to delete on its own initiative, to tidy up or because a skill looks unused. 1.2's fingerprint (shipped in the 2.3.0 betas) is recorded as a literal so untouched copies upgrade silently.

No code gate: the request lives in the conversation, so only the model can attest it, with or without a flag. Core skills stay undeletable, and a skill must still be attached. AGENTS.md is updated to match.

How I tested it

bun run check, format, lint, and the full unit suite (2004) pass, including the shipped-skill history test. The behaviour change is model-facing wording; Leo judges it live.

AI assistance: Claude Code (Fable 5.1) wrote the change from Leo's brief (drop the agent/user skill distinction; the agent deletes a skill only when the user asks); Leo reviews and tests live.

Checklist

  • bun run check, bun run format, bun run lint, and bun run test pass locally
  • I tried the change in a real Obsidian vault (or explained above why that isn't applicable)
  • I read CONTRIBUTING.md, including the section on AI assistance
  • If this adds a provider, a bundled skill, a built-in tool, or changes manifest.json: I noted that the docs site needs updating (see "Documentation" in CONTRIBUTING.md) — the skills page's delete sentence is updated in docs(skills): the agent deletes a skill only when you ask site#7

@greptile-apps

greptile-apps Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Medium risk] Changes agent skill deletion behavior and documentation.

The PR does not appear safe to merge while an unprompted tool call can delete an attached user-written or integration skill.

Summary

The latest changes replace the author-based delete override with instructions that deletion should occur only at the user’s request.

  • Tool descriptions, the delete schema description, bundled skill guidance, and repository documentation now state the request-only rule.
  • The delete path no longer checks the skill’s author or a user-request flag, leaving that rule unenforced by the tool.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Delete skill call] --> B{Skill exists?}
  B -- No --> X[Refuse]
  B -- Yes --> C{Core skill?}
  C -- Yes --> X
  C -- No --> D{Attached to agent?}
  D -- No --> X
  D -- Yes --> E[Recursively delete skill folder]
Loading

Reviews (2) · Last reviewed commit: "fix(skills): delete a skill only when th..."

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>
@Leo310
Leo310 force-pushed the fix/manage-skills-delete-authorship branch from 3e30674 to 1d1b0a1 Compare September 28, 2026 07:31
@Leo310 Leo310 changed the title fix(skills): delete only skills the agent created, unless the user asks fix(skills): delete a skill only when the user asks for it Sep 28, 2026
@greptile-apps

greptile-apps Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff

These findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.

  • P1 Unrequested skill deletion src/agent/tools/manageSkills.ts:324 ▶

    When the agent calls delete for an attached, non-core skill it did not create, the removed author and userRequested checks no longer stop the call. The tool deletes the entire folder immediately, even if the user did not ask, so an unprompted call can remove a user-written or integration skill. The new request-only wording is guidance to the agent, not a check in this path.

@Leo310

Leo310 commented Sep 28, 2026

Copy link
Copy Markdown
Member Author

Re Unrequested skill deletion (manageSkills.ts:324): deliberate, as the description says. The earlier author check plus userRequested flag was removed on purpose. Authorship doesn't track ownership here: skills are mostly written by the user and the agent together, so the stamp would let the agent delete a skill the user refined while protecting one the agent rewrote many times. The flag was also no check in practice: only the model can attest that the user asked, since the request lives in the conversation, so the flag and the guidance protect equally. And nothing in the plugin deletes skills autonomously any more (the post-turn review is gone, #532). The guards that are enforceable stay: core skills can't be deleted and the skill must be attached.

@Leo310
Leo310 merged commit 00a2f29 into main Sep 28, 2026
3 checks passed
@Leo310
Leo310 deleted the fix/manage-skills-delete-authorship branch September 28, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant