Skip to content

Feature discussion: safe skill editing over MCP with revision checks #540

Closed

Description

@salmonumbrella

The situation

Following up on #428, can you consider an opt-in way for a remote assistant to edit a skill through MCP, with revision checks and a reviewable change?

The read side is working in my setup through Executor. The connected Skillshare tools currently expose list_skills and read_skill. The next use case is: I explicitly ask the assistant to improve a skill, it can read the skill remotely, but it has no supported way to save the change back. When my usual editing computer is offline, going back to a local editor is not an available workaround.

I'd like to keep Skillshare's canonical source as the source of truth and have the change reach the normal sync flow. If that source itself is offline, the system should distinguish a staged proposal from a persisted change; it shouldn't claim a save succeeded.

What needs planning

Revision checks seem essential: read a skill at a known revision, propose a change against that revision, and reject the write with a clear conflict if something changed in the meantime. A stale preview or approval should require another review, rather than silently overwriting a newer edit.

Other questions I'd like your guidance on:

  • Permissions: opt-in writes, scoped to selected skills or source roots, with read-only serving still the default. Preserve path and symlink containment, and keep saving content separate from executing bundled scripts.
  • Review and commit: show the exact diff before applying it. If an edit touches SKILL.md and supporting files, should the package be validated and committed atomically?
  • Verification: return the new revision and support readback of the saved package, so the assistant can verify the result.
  • History: how should revisions, rollback and an audit trail fit with Skillshare's existing Git workflow?
  • Sync and provenance: how should edits interact with imported upstream skills, later updates, target filters and concurrent edits from another computer? How do we avoid changing a generated target copy or losing a local customization?
  • Validation: reuse the existing package/frontmatter validation, size limits and audit checks where appropriate, with clear failures before anything is published.

These are design questions rather than a request to ship every feature at once. A small, safe first version would be useful.

Inspiration

Executor's Add managed Agent Skills PR #2091 proposes immutable package revisions, editable package files, source-update diffs and retained source references. It builds on Add Agent Skills #1986, which proposes a skill save API and MCP delivery. Both PRs are currently closed without merging, so I'm pointing to the designs as inspiration, not claiming those capabilities shipped or that they solve MCP editing.

Ask

Would you consider designing and implementing this in Skillshare? Where do you think the write API should live: alongside mcp serve, in a separate management API exposed through MCP, or in a Git-backed proposal/review workflow?

I'd especially like your view on the ideal setup for remote assistants and an occasionally offline editing computer, while keeping one canonical source and a clear approval/conflict model. #428 deliberately left writes out of scope; this is a separate follow-up to discuss that next step.

Activity

  1. self-assigned this
    on Oct 12, 2026
  2. runkids commented on Oct 12, 2026

    @runkids
    Owner

    Thanks for laying this out so clearly. I haven't decided anything yet, and I'd like to agree on the basics with you before going further.

    My initial thinking:

    • If we add writes, mcp serve seems like the natural place, since it already runs next to your canonical source. That also helps your offline case: if that machine is offline, the edit fails rather than looking like it succeeded.
    • I'd want to start very small: edit one existing file at a time, reject the edit if the file changed since it was read, and run the usual checks before saving.
    • Skills installed from upstream are tricky, because update can overwrite local edits. I'd keep those out of scope at first.

    Does that direction make sense to you? Two things I'm curious about:

    1. Is editing one file at a time enough for how you'd use it?
    2. When Executor asks you to approve a tool call, does it show the content being written?
  3. salmonumbrella commented on Oct 12, 2026

    @salmonumbrella
    ContributorAuthor

    Yes, that direction makes sense.

    1. Editing one existing file at a time covers my immediate use case. Atomic multi-file edits would be useful later when a change spans SKILL.md and supporting files, but they can wait. Keeping upstream-installed skills out of the first version makes sense too. And if the canonical source is offline, a clear failure is the right behavior.

    2. On Executor approval, I checked the source rather than testing my deployed approval screen. Its browser approval page renders the tool arguments as scrollable JSON, but the generic approval message truncates its argument preview at 4,000 characters. Approval also depends on the client/mode, so I can't confirm that my current setup always shows the full content being written.

    I'd suggest making a full content/diff preview available and tying the reviewed change to the exact file, base revision and proposed content, then rechecking that revision when saving. That would avoid relying on a possibly truncated approval prompt and catch changes made while approval is pending.

  4. runkids commented on Oct 13, 2026

    @runkids
    Owner

    Thanks for checking Executor's source. The 4,000-character truncation settles it for me: the client's approval prompt can't be the review step. The server has to show the full change and bind the save to exactly what was shown. Here is a full proposal for a first version, so we can agree on it before any code is written.

    Proposal: opt-in single-file skill edits over mcp serve

    Goals

    • A remote assistant can change one existing file of a local skill, through the mcp serve instance that runs next to the canonical source.
    • Every save is tied to the exact file, base revision and content that was previewed.
    • A file that changed in the meantime is never overwritten: the save fails with a conflict.
    • If the source machine is offline, the call fails. Nothing is staged elsewhere, and nothing claims to be saved.

    Not in the first version

    • Creating, deleting or renaming files, and multi-file edits.
    • Skills installed or tracked from upstream (anything update can overwrite).
    • Committing to Git, rollback, or proposal queues.
    • Writing to target copies. Only the source is written; targets get the change through the normal sync.

    1. Revisions

    mcp serve already lists every skill file with a sha256: digest in the catalog, and refuses a read whose content no longer matches it. That digest becomes the revision:

    • read_skill also returns the revision of the file it read.
    • The revision is a content hash, not a timestamp, so cp -p, rsync -t or a sync from another computer can't hide a change.

    2. Turning writes on

    Writes are off unless the server is started with them, for example:

    skillshare mcp serve --allow-edit 'my-skill' --allow-edit 'drafts/*'
    
    • Without --allow-edit, the server stays read-only and the edit tools are not listed at all.
    • Each pattern names skills by their source path. Only listed skills are writable, and only if they are not upstream-installed or tracked.
    • A target filter (--target) still applies: a skill the server doesn't serve can't be edited.
    • The existing HTTP rules stay: off loopback, a bearer token and TLS are required.

    3. Two tools

    preview_skill_edit — writes nothing.

    • Input: the file URI from read_skill, its base revision, and the full new content.
    • Checks: the skill is writable, the file exists and is listed, the base revision still matches, the content is valid UTF-8 text within the size limit, and, for SKILL.md, the frontmatter still validates.
    • Runs the content audit on the new content and reports any findings.
    • Returns the full unified diff, the audit result, and a preview_id.

    apply_skill_edit — the only tool that writes.

    • Input: only the preview_id. The content can't be changed between preview and save.
    • The server holds the previewed file, base revision and content. Before writing, it checks the base revision again. If the file changed while approval was pending, it returns a conflict and asks for a new preview.
    • Writes atomically (temporary file, then rename), confined to the skill directory, and refuses links on the path, the same way the dashboard editor saves skills.
    • Returns the new revision. The assistant can then call read_skill and compare.
    • A preview expires after a short time (say 10 minutes) and can be applied once. Restarting the server drops all previews.

    The diff the server returns is the review. If a client truncates its approval prompt, the assistant still has the full diff from preview_skill_edit to show you.

    4. Failures

    Each failure names its cause and says nothing was written:

    • conflict: the file changed since it was read; read it again.
    • not_writable: the skill isn't in --allow-edit, or it is installed from upstream.
    • invalid: frontmatter, encoding or size check failed.
    • audit_blocked: the audit found something at or above the blocking threshold. The edit tools never bypass the audit.
    • expired: the preview is older than the limit or was already applied.

    5. History and sync

    • Each save goes into the operation log (skillshare log) with the skill, file, old and new revision, and that it came from MCP.
    • If your source is a Git repository, the change shows up like any local edit. You commit it with your normal workflow (skillshare commit / push). The server doesn't commit.
    • Targets update on the next sync, as for any source edit.
    • Saving never runs scripts bundled in the skill.

    Questions for you

    1. Is a startup flag listing writable skills the right control for your setup, or would you rather keep the list in the config file?
    2. Should an audit finding below the blocking threshold be shown in the preview only, or should it also stop the save?
    3. Is 10 minutes enough time between preview and approval in your Executor setup?

    I haven't scheduled the work yet. If this matches what you had in mind, I'll turn it into a tracked plan and start with sections 1 to 3.

  5. salmonumbrella commented on Oct 13, 2026

    @salmonumbrella
    ContributorAuthor

    Thanks, this matches what I had in mind. Sections 1 to 3 as the first slice sounds right to me.

    Some context on why I still want this. I'm planning to move my Executor to v2. It ships skills inside each app, and those get their own Git-backed revisions and deploys. That covers app-specific instructions. The general skills I sync to every agent stay in Skillshare, so this edit path is still the only way a remote assistant can change them.

    1. Flag or config. I'd rather keep the list in the config file. In my setup the server's command line lives in the Executor deployment, while the config is in the same Git repo as the skills. If the list lives there, changing what's writable is a reviewed commit rather than a redeploy. It also sits next to the target filter it's checked against. A flag on top is fine if you want one, but I wouldn't need it.

    2. Audit findings below the threshold. I'd show them in the preview only and let the save go through. audit.block_threshold is already the policy setting, and a second gate below it would make that setting mean less. The approval also isn't always a person. In v2, scheduled runs approve automatically by default, so the blocking threshold is the only check that always runs. If it's cheap, marking which findings are new compared with the base revision would make the preview easier to read.

    3. Preview lifetime. 10 minutes is a bit short. In v2, a paused approval waits up to 15 minutes by default, and the clock starts after the preview already exists. A 10-minute preview can expire while an approval is still valid, and then the approved apply fails. Because apply rechecks the base revision anyway, a longer lifetime costs little. I'd default to 30 minutes and make it configurable.

    One request from the Executor side: please set MCP tool annotations. v2 treats any MCP tool without readOnlyHint: true as a mutation. An MCP server added from the dashboard asks for approval only before tools marked destructiveHint: true. list_skills and read_skill don't set annotations today. If list_skills, read_skill and preview_skill_edit were marked read-only and apply_skill_edit destructive, the default setup would ask exactly once, at the apply step.

    Also, from reading the code (I haven't tested it), v2 self-host doesn't look like it can run stdio MCP servers: its stdio helper requires a Node-compatible host. So I'd be using mcp serve --http with the bearer token and TLS, and the HTTP rules you kept are the ones that matter for me.

  6. salmonumbrella commented on Oct 13, 2026

    @salmonumbrella
    ContributorAuthor

    One more idea, separate from the edit work. I'm happy to move it to its own issue if you'd prefer.

    I'd like Skillshare and Executor to read the same Git-backed source, so someone can use both without keeping two copies of their skills. Executor v2 already has two features Skillshare could build on:

    1. Skillshare → Executor. v2 apps can load skills from a published index with wellKnownSkills. The index is /.well-known/agent-skills/index.json, shaped { "skills": [{ "name", "version", "files": [...] }] }, and each file is fetched from <name>/<path> next to it. When every entry has a version, Executor checks for changes with one request and reloads only when something changed. mcp serve --http already builds a catalog with a SHA-256 digest for each file. It could serve this index from that catalog and derive version from those digests. An edit saved through apply_skill_edit would then reach Executor on its next check, with no separate publish step.

    2. Executor → Skillshare. A v2 app's source is a plain Git repo, on both hosted and self-hosted Executor. Skills shipped inside an app could then be installed and tracked like any other repo. I think this already works with a Git credential helper. I'll test it and report back.

    From checking my own source (about 460 skills and 2,600 files), the index would need to handle a few things:

    • Executor needs flat, unique, lowercase-hyphen names that match the frontmatter. 12 of my names appear more than once, mostly per-agent variants of the same skill, and 2 don't fit the rules. Skillshare would have to skip and list those, the way mcp serve --check reports skipped skills.
    • Executor caps one index at 1,000 files and 20 MB, so a single index of my whole source wouldn't load. One index per target would let people split a large source, and --target already does that scoping.
    • I still need to check whether Executor's loader can send the bearer token the HTTP server requires.

    Does this fit how you see mcp serve, or would you rather make it a separate command?

  7. runkids commented on Oct 13, 2026

    @runkids
    Owner

    Thanks, these answers all make sense. Here is the plan with them folded in.

    1. Writable skills live in the config file. No flag. Roughly like this (the key names are not final):

    mcp:
      serve:
        edit:
          skills: ["my-skill", "drafts/*"]
          preview_ttl: 30m

    Patterns match skill source paths, the same way the target filter does, and the target filter is still applied first. Upstream-installed and tracked skills stay read-only even if a pattern matches them.

    2. Findings below audit.block_threshold are shown, not enforced. The preview audits both the base content and the new content, and marks each finding as new or already present. Findings are matched by rule and matched text, not line number, so an edit higher in the file doesn't make old findings look new. Anything at or above the threshold still blocks the save.

    3. Previews last 30 minutes by default, configurable as above. Apply still rechecks the base revision, so a long-lived preview can't overwrite a newer edit.

    Tool annotations. Agreed:

    Tool Annotation
    list_skills, read_skill readOnlyHint: true
    preview_skill_edit readOnlyHint: true (it writes nothing)
    apply_skill_edit destructiveHint: true

    The two read tools can get their annotations on their own, ahead of the edit work.

    The flow with your --http setup:

    sequenceDiagram
        autonumber
        participant A as Assistant
        participant E as Executor
        participant S as skillshare mcp serve
        participant F as Skills source
    
        A->>S: read_skill(uri)
        S->>F: read file
        S-->>A: content + revision (sha256)
    
        A->>S: preview_skill_edit(uri, base revision, new content)
        Note over S: read-only, so no approval prompt
        S->>F: check skill is writable and revision still matches
        S->>S: validate, audit base and new content
        alt check fails or audit blocks
            S-->>A: error, nothing written
        else ok
            S->>S: hold preview in memory (30 min)
            S-->>A: full diff + audit result (new / existing) + preview_id
        end
    
        A->>E: apply_skill_edit(preview_id)
        Note over E: destructiveHint, so ask once
        E->>E: approval (person or automatic)
        E->>S: apply_skill_edit(preview_id)
        S->>F: recheck revision
        alt file changed since preview
            S-->>A: conflict, read again and preview again
        else preview expired or already applied
            S-->>A: expired, preview again
        else unchanged
            S->>F: atomic write, no links on path
            S->>S: record in operation log
            S-->>A: new revision
            A->>S: read_skill(uri) to verify
        end
        Note over F: targets update on the next sync
    
    Loading

    Previews are held in the server's memory, so they work across stateless HTTP requests, and a restart drops them.

    The .well-known index: yes, please open it as its own issue. It fits next to mcp serve --http, since the catalog and digests already exist, but it has its own questions: name rules, the 1,000-file cap and whether Executor's loader can send the bearer token. Your numbers from a 460-skill source will help there, and so will your result on installing an Executor app repo.

  8. salmonumbrella commented on Oct 14, 2026

    @salmonumbrella
    ContributorAuthor

    Great, thank you for working on it @runkids

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

featureNew feature or request

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions