Skip to content

feat(skill): decouple read-only skills from Box execution - #2512

Open
huanghuoguoguo wants to merge 6 commits into
masterfrom
feat/2410-sdk-skill-store
Open

huanghuoguoguo wants to merge 6 commits into
masterfrom
feat/2410-sdk-skill-store

Conversation

@huanghuoguoguo

@huanghuoguoguo huanghuoguoguo commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Closes #2410

Paired SDK change: langbot-app/langbot-plugin-sdk#128

Summary

  • Make Core SkillRepository the owner of Skill catalog, CRUD, activation snapshots, and read-only resources.
  • Keep activation and resource reads available when Box is disabled; require Box only for execution and sandbox-backed mutation.
  • Compose generic read-only mounts in Core so Box has no Skill semantics.
  • Use Workspace-scoped repository locks so unrelated tenants do not serialize Skill I/O.
  • Retain only the persisted skills-root fallback needed for existing installation data; mixed Core/SDK protocol compatibility is intentionally unsupported.

Release ordering

Core and the Box Runtime SDK are deployed as one release unit, but the repository dependency must be published in order: merge and publish SDK PR #128 first, then bump the langbot-plugin pin in this PR from 0.5.7 to that release. No runtime mixed-version bridge is retained.

Validation

  • Ruff and frontend checks pass.
  • Current Python and integration jobs stop during collection because the repository still pins PyPI langbot-plugin 0.5.7, which does not yet contain langbot_plugin.skill_store. They should be rerun after the paired SDK release and pin bump.
  • Paired SDK Box and SkillStore suite: 463 passed.

@huanghuoguoguo
huanghuoguoguo marked this pull request as ready for review September 6, 2026 13:59
@RockChinQ

Copy link
Copy Markdown
Member

@dadachann Analyze and check if this PR is okay for merging, if the design is good, fix the CI errors.

@dadachann

Copy link
Copy Markdown
Collaborator

👋 I’m dadachann, the LangBot community maintenance agent. I’ve picked up the review/task on this pull request and will share findings and next steps here.

@RockChinQ

Copy link
Copy Markdown
Member

We agree with moving centralized Skill management into Core. To complete this separation, we propose the following consistency contract for the paired Core/SDK changes. These are design requirements, not claims that the current PR already implements them.

Ownership and publication

  • Core owns the authoritative Skill registry: metadata, the current revision pointer, and immutable published package revisions. Manual installations also publish into this registry.
  • A sandbox consumes a specific published revision. Agent edits happen in a separate writable working copy; they must not mutate the published package or implicitly synchronize back to Core.
  • Agents explicitly call registration/update tools to publish their changes. Updates include base_revision; Core rejects conflicting updates instead of silently overwriting another writer's changes.
  • Publish into a temporary location, validate the complete package and compute its content digest, then atomically publish the revision and advance the current pointer. Compute the digest at publication rather than scanning all package contents on every resource read. Multiple Core writers need storage-level concurrency control, not only process-local locks.

Activation and sandbox materialization

  • Activation pins a revision for the run. Instructions, resource reads, and script execution must all use that same revision.
  • Activation remains available without Box. Materialize the pinned package when execution needs it; activation alone must not require creating a sandbox.
  • Use a read-only mount when the execution node can access the same immutable revision directory and the backend supports reliable read-only mounts.
  • Otherwise copy/download that revision into a dedicated sandbox package directory. Verify the complete package before marking it ready. Failed or partial transfers must block execution; do not silently continue with stale or mixed files.
  • Keep published packages separate from writable drafts, for example /skills/<skill-id>/<revision>/ versus /workspace/skill-drafts/<draft-id>/. A copied execution package must provide the same immutability guarantee as a read-only mount; if a backend cannot enforce this, it must not claim that guarantee.
  • New sandboxes should materialize only the Skills activated and authorized for the run, rather than every pipeline-bound Skill.

Updates and sandbox reuse

  • Publishing v2 notifies sandboxes using the Skill that a new revision is available; they may prefetch it. It must not overwrite v1 underneath an active run. Existing runs continue on v1; subsequent runs resolve v2.
  • Notifications accelerate refresh but are not the correctness mechanism. Before execution, reconcile the requested revision with the sandbox's prepared package manifest, so offline sandboxes and missed notifications are safe.
  • Sandbox reuse must account for the pinned revisions and mount set. Concurrent runs must not switch each other's package versions. Keep writable Workspace data separate from disposable execution containers, and scope dependency caches by revision and relevant environment inputs.
  • Docker cannot add bind mounts to an existing container. When its mount set changes, safely rebuild/select an execution container while preserving Workspace data; do not destroy one with active work. Remote/copy-based backends prepare a fresh verified revision directory instead of incrementally overwriting a live package.

Restart, deletion, and recovery

  • Persist the current revision pointer, publication manifests, and revision bindings for recoverable runs. After Core/Box restart, validate the sandbox manifest against those bindings or recreate the execution environment.
  • Never restore a run by Skill name alone and silently resolve the latest revision. If the pinned revision is unavailable, return an explicit recovery error.
  • Normal deletion prevents new use, while revisions referenced by active/recoverable runs remain until safe garbage collection. Emergency revocation is a separate explicit operation that rejects or terminates affected execution.

Gaps to address in the current implementation

The current stat-v1 token detects metadata changes but does not provide an immutable package snapshot. Execution mounts use mutable package paths, and the Box mount signature compares paths/modes rather than package contents. This can pair old activation instructions with newly modified scripts. The E2B best-effort upload path can also leave partial uploads or obsolete remote files. These paths need the same revision contract described above.

Minimum acceptance coverage should include: activating v1 then publishing v2 still reads/executes v1 in the existing run; a new run uses v2; interrupted transfer never executes partial content; Core-only and Box-only restarts preserve or explicitly reject the pinned revision; concurrent runs do not switch each other's versions; conflicting updates are rejected; and deleted files do not survive in a newly prepared revision.

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.

[Feature]: Decouple Skill activation and read-only resources from Box sandbox execution

3 participants