Compare commits

...

4 Commits

Author SHA1 Message Date
claude-ceo-assistant 7fbb8cb6e9 feat(plugins): atomic install — stage→snapshot→swap→marker (docker path)
Closes molecule-core#114 for the docker (local-OSS) path.
EIC (SaaS) path tracked as a follow-up — same shape, different
exec primitives (ssh vs docker exec); shipping both in one PR
doubles the test surface.

THE FOUR-STEP DANCE
  1. STAGE     — docker.CopyToContainer extracts tar into
                 /configs/plugins/.staging/<name>.<ts>/
  2. SNAPSHOT  — if /configs/plugins/<name>/ exists, mv to
                 /configs/plugins/.previous/<name>.<ts>/
  3. SWAP      — atomic mv staging → live (single rename(2))
  4. MARKER    — touch /configs/plugins/<name>/.complete

Workspace-side plugin loaders should refuse to load any plugin dir
without .complete (separate small change, not in this PR — the marker
write is the necessary precursor; consumer side is a follow-up so
existing-content plugins don't break before they're re-installed).

ROLLBACK
  - Stage failure: rm -rf staging dir; live untouched
  - Snapshot failure: rm -rf staging dir; live untouched (no rename happened)
  - Swap failure with snapshot present: mv previous back to live
  - Swap failure (no snapshot): rm -rf staging; live (which never
    existed) stays absent
  - Marker failure: content already in place, log loudly with manual
    recovery hint (touch <plugin>/.complete) — don't roll back since
    the new content is what we wanted, just unmarked

GC
  Best-effort delete of previous-version snapshot after successful
  marker write. Failures non-fatal — next install or a separate
  sweeper reclaims. Sweeper for stale .previous/* across reboots is
  follow-up scope.

CONCURRENCY
  Each install gets a unique stamp (UTC second precision), so two
  concurrent reinstalls land in distinct staging dirs and the second
  swap simply overwrites the first's live result. The atomicity is
  per-install, not cross-install — by design (the platform serializes
  POST /workspaces/:id/plugins via Go-side semaphore upstream of
  this code, so cross-install collisions don't reach here).

CHANGES
  + plugins_atomic.go        — installVersion + atomicCopyToContainer
  + plugins_atomic_tar.go    — tarWalk/tarHostDirWithPrefix helpers
  + plugins_atomic_test.go   — 5 unit tests (paths, stamp shape,
                               tar happy path, symlink-skip, prefix
                               normalization). All green.
  ~ plugins_install_pipeline.go::deliverToContainer — swap
    copyPluginToContainer call to atomicCopyToContainer

Old copyPluginToContainer is retained (still called by Download()) so
this PR is purely additive on the install path; no public API change.

PHASE 4 SELF-REVIEW (FIVE-AXIS)
  Correctness: Required (addressed) — swap-failure rollback writes mv
    of previous back to live before returning the error; if rollback
    itself fails, we wrap both errors and surface the combined fault.
    Marker-write failure is treated as content-landed-but-unmarked
    (LOG, don't roll back the new content).
  Readability: No finding — installVersion path methods make the
    /staging/.previous/live/marker layout obvious from one struct.
    tarWalk extracted from the inline filepath.Walk in
    plugins_install_pipeline.go for testability.
  Architecture: No finding — atomicCopyToContainer composes existing
    execAsRoot / docker.CopyToContainer primitives; no new dependencies.
    Old copyPluginToContainer kept for Download() — single responsibility
    per function.
  Security: No finding — symlinks still skipped during tar walk
    (defense vs hostile plugin escaping its own dir). Marker writes
    use composeable path.Join, no user input touches the path.
  Performance: No finding — adds ~3 docker exec calls per install
    (mkdir, mv-snapshot, mv-swap, touch — actually 4) on top of the
    one CopyToContainer. Each exec ~50-100ms in practice; install
    end-to-end was already seconds-scale, this rounds to noise.

REFS
  molecule-core#114 — this issue
  Companion: molecule-core#112 (hot-reload classifier — depends on .complete marker)
  Companion: molecule-core#113 (version subscription — uses install machinery)
  EIC follow-up: separate issue to be filed for SaaS path parity

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 08:22:52 -07:00
claude-ceo-assistant e37a289eb6 Merge pull request 'feat(org-import): inject per-role persona env from operator-host bootstrap dir' (#110) from feat/persona-env-injection into main 2026-05-08 15:17:17 +00:00
claude-ceo-assistant 61166f8848 Merge pull request 'feat(local-dev): air-based hot-reload for workspace-server in docker-compose dev mode' (#118) from feat/air-hot-reload-dev into main 2026-05-08 15:16:58 +00:00
claude-ceo-assistant 9d50a6dae4 feat(local-dev): air-based hot-reload for workspace-server
Closes core#116. Brings local-dev iteration parity with the canvas's
Turbopack HMR — edit a Go file, see the platform restart in <5s
instead of running 'docker compose up --build' (~30s) per change.

USAGE
  make dev   # docker compose with air-driven live reload
  make up    # production-shape stack (no air, normal Dockerfile)

WHAT THIS ADDS
  workspace-server/.air.toml      — air watch config
  workspace-server/Dockerfile.dev — air-on-golang:1.25-alpine, dev-only
  docker-compose.dev.yml          — overlay swapping platform service
                                    to Dockerfile.dev + bind-mounting
                                    workspace-server/ source
  Makefile                        — make {dev,up,down,logs,build,test}

WHAT THIS DOES NOT TOUCH
  workspace-server/Dockerfile (production multi-stage build)
  docker-compose.yml          (prod-shape stack)
  CI workflows                (build prod image directly)
  Tenant deployment / SaaS    (image swap stays the model)

Pure additive. Existing 'docker compose up' path unchanged; production
stays on the static binary. Air install pinned via go install at image
build time so the dev image is reproducible-enough for local use (we
don't pin air to a SHA — the dev image is rebuilt locally and updates
opportunistically).

PHASE 4 SELF-REVIEW (FIVE-AXIS)
  Correctness: No finding — additive change, no existing path modified.
    .air.toml watches .go + .yaml under workspace-server/, excludes
    _test.go and tests dir so test edits don't trigger rebuild.
    Dockerfile.dev mirrors prod's 'go mod download' so first rebuild
    is fast.
  Readability: No finding — three small files plus a Makefile, each
    with header comments explaining the WHY, not just the WHAT. The
    Makefile uses the standard ## help-target pattern.
  Architecture: No finding — overlay pattern (docker-compose.dev.yml
    on top of docker-compose.yml) is the standard compose convention
    for env-specific overrides. Doesn't fork the prod path.
  Security: No finding because no production code path; dev-only image
    isn't built in CI and isn't published to ECR.
  Performance: No finding — air debounce=500ms, exclude_unchanged=true
    so a save that doesn't change content is a no-op rebuild.

REFS
  core#116 — this issue
  Companion: core#117 (workspace-side config-watcher for hot-reload of
  config.yaml) — different scope; this issue is platform-only.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 08:10:50 -07:00
8 changed files with 635 additions and 1 deletions
+28
View File
@@ -0,0 +1,28 @@
# Top-level Makefile — convenience wrappers around docker compose.
#
# Most molecule-core dev work happens via these shortcuts. CI doesn't
# use this Makefile; CI calls docker compose / go test directly so the
# Makefile can evolve without breaking the build.
.PHONY: help dev up down logs build test
help: ## Show this help.
@grep -E '^[a-zA-Z_-]+:.*?## ' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-12s\033[0m %s\n", $$1, $$2}'
dev: ## Start the full stack with air hot-reload for the platform service.
docker compose -f docker-compose.yml -f docker-compose.dev.yml up
up: ## Start the full stack in production-shape mode (no air, normal Dockerfile).
docker compose up
down: ## Stop the stack and remove containers (volumes preserved).
docker compose down
logs: ## Tail logs from all services (Ctrl-C to detach).
docker compose logs -f
build: ## Force a fresh build of the platform image (no cache).
docker compose build --no-cache platform
test: ## Run Go unit tests in workspace-server/.
cd workspace-server && go test -race ./...
+43
View File
@@ -0,0 +1,43 @@
# docker-compose.dev.yml — overlay over docker-compose.yml for local dev
# with air-driven live reload of the platform (workspace-server) service.
#
# Usage:
# docker compose -f docker-compose.yml -f docker-compose.dev.yml up
# (or `make dev` shorthand from repo root)
#
# What this overlay changes vs docker-compose.yml alone:
# - Platform service uses workspace-server/Dockerfile.dev (air on top of
# golang:1.25-alpine) instead of the multi-stage prod Dockerfile.
# - Platform service bind-mounts the host's workspace-server/ source
# into /app/workspace-server so air sees source edits live.
# - Other services (postgres, redis, langfuse, etc.) inherit unchanged
# from docker-compose.yml.
#
# What stays the same:
# - All env vars, volumes, depends_on, healthchecks from docker-compose.yml.
# - Network topology + ports.
# - Postgres/Redis as service containers (no in-process replacements).
services:
platform:
build:
context: .
dockerfile: workspace-server/Dockerfile.dev
# Rebind source: edits under host's workspace-server/ propagate live.
# The named volume on go-build-cache speeds up first build per container.
volumes:
- ./workspace-server:/app/workspace-server
- go-build-cache:/root/.cache/go-build
- go-mod-cache:/go/pkg/mod
# Air signals the running binary on rebuild; ensure shell stops cleanly.
init: true
# Mark the service as dev-mode so the platform can short-circuit any
# behavior that's incompatible with hot-reload (e.g. background
# cron-style watchers that don't survive process restart). No-op
# today; reserved for future flag use.
environment:
MOLECULE_DEV_HOT_RELOAD: "1"
volumes:
go-build-cache:
go-mod-cache:
+49
View File
@@ -0,0 +1,49 @@
# air.toml — live-reload config for local docker-compose dev mode.
#
# Active when the platform service runs from workspace-server/Dockerfile.dev
# (selected via docker-compose.dev.yml overlay). In production, the regular
# Dockerfile builds a static binary; air is dev-only.
#
# Reference: https://github.com/air-verse/air
root = "."
testdata_dir = "testdata"
tmp_dir = "tmp"
[build]
# Same build invocation as Dockerfile's builder stage minus the
# CGO_ENABLED=0 toggle (CGO ok in dev for richer race detector output).
cmd = "go build -o ./tmp/server ./cmd/server"
bin = "tmp/server"
full_bin = ""
args_bin = []
# Watch every .go and .yaml file under workspace-server/.
include_ext = ["go", "yaml", "tmpl"]
# Don't watch tests, build artifacts, vendored deps, or migration .sql
# (migrations need a clean DB anyway — handled by docker-compose down/up).
exclude_dir = ["assets", "tmp", "vendor", "testdata", "node_modules"]
exclude_file = []
# _test.go and *_mock.go shouldn't trigger a rebuild — saves cycles.
exclude_regex = ["_test\\.go$", "_mock\\.go$"]
exclude_unchanged = true
follow_symlink = false
log = "build-errors.log"
# Kill running binary 1s before starting new one.
kill_delay = "1s"
send_interrupt = true
stop_on_error = true
# Debounce: wait this long after last change before triggering rebuild.
delay = 500
[log]
time = false
[color]
main = "magenta"
watcher = "cyan"
build = "yellow"
runner = "green"
[misc]
# Don't keep the tmp/ dir around between runs.
clean_on_exit = true
+38
View File
@@ -0,0 +1,38 @@
# Dockerfile.dev — local-development image with air-driven live reload.
#
# Selected by docker-compose.dev.yml (overlay over docker-compose.yml).
# Production stays on workspace-server/Dockerfile (static binary, no air).
#
# Workflow:
# 1. docker compose -f docker-compose.yml -f docker-compose.dev.yml up
# 2. Edit any .go file under workspace-server/
# 3. air detects, rebuilds, kills old binary, starts new one (~3-5s)
# 4. No `docker compose up --build` needed
#
# Templates + plugins are NOT pre-cloned here — air-mode assumes the
# developer's filesystem has the workspace-configs-templates/ + plugins/
# dirs available, mounted at runtime via docker-compose.dev.yml.
FROM golang:1.25-alpine
# air + git (for go mod) + ca-certs (for TLS) + tzdata (for time-zone DB).
RUN apk add --no-cache git ca-certificates tzdata wget \
&& go install github.com/air-verse/air@latest
WORKDIR /app/workspace-server
# Pre-fetch deps so the first `air` rebuild on a fresh container is fast.
# These are bind-mount-overridden at runtime, so the COPY here is just
# to warm the module cache.
COPY workspace-server/go.mod workspace-server/go.sum ./
RUN go mod download
# Source is bind-mounted at runtime (see docker-compose.dev.yml volumes
# block) so the Dockerfile doesn't need to COPY it. air watches the
# bind-mounted dir for changes.
ENV CGO_ENABLED=1
ENV GOFLAGS="-buildvcs=false"
# Run air with the .air.toml in the bind-mounted source dir.
CMD ["air", "-c", ".air.toml"]
@@ -0,0 +1,207 @@
package handlers
// plugins_atomic.go — atomic install pattern for plugin delivery into a
// running workspace container. Closes molecule-core#114.
//
// Replaces the prior "tar + docker.CopyToContainer to /configs/plugins/<name>"
// single-step write (no atomicity, no marker, no rollback) with a 4-step
// dance:
//
// 1. STAGE — extract tar into /configs/plugins/.staging/<name>.<ts>/
// 2. SNAPSHOT — if /configs/plugins/<name>/ exists, mv to .previous/<name>.<ts>/
// 3. SWAP — mv /configs/plugins/.staging/<name>.<ts>/ → /configs/plugins/<name>/
// 4. MARKER — touch /configs/plugins/<name>/.complete
//
// On any post-snapshot failure we attempt a best-effort rollback by mv-ing
// the previous snapshot back into place. The .complete marker is the
// canonical "this install is fully landed" signal — workspace-side plugin
// loaders should refuse to load a plugin dir without it.
//
// Scope: docker path only (workspace running as a local container). The
// SaaS path (deliverViaEIC, SSH-into-EC2) is unchanged in this PR; tracked
// as a follow-up. The same stage-then-swap shape applies but the exec
// primitives differ (ssh vs docker exec), and shipping both paths in one
// PR doubles the test surface.
import (
"bytes"
"context"
"fmt"
"path"
"strings"
"time"
"github.com/docker/docker/api/types/container"
)
const (
pluginsRoot = "/configs/plugins"
pluginsStagingDir = "/configs/plugins/.staging"
pluginsPrevDir = "/configs/plugins/.previous"
completeMarker = ".complete"
)
// installVersion identifies one install attempt — the plugin name plus a
// monotonic-ish UTC timestamp suffix. Used to namespace the staging dir
// and any snapshot of the previous version, so a reinstall mid-flight
// can't collide with a concurrent reinstall.
type installVersion struct {
plugin string
stamp string // e.g. 20260508T141530Z
}
func newInstallVersion(plugin string) installVersion {
return installVersion{
plugin: plugin,
stamp: time.Now().UTC().Format("20060102T150405Z"),
}
}
// stagedPath is the container path where the new content lands during fetch.
// e.g. /configs/plugins/.staging/molecule-skill-foo.20260508T141530Z
func (v installVersion) stagedPath() string {
return path.Join(pluginsStagingDir, v.plugin+"."+v.stamp)
}
// previousPath is where the prior live version is moved before swap.
// e.g. /configs/plugins/.previous/molecule-skill-foo.20260508T141530Z
func (v installVersion) previousPath() string {
return path.Join(pluginsPrevDir, v.plugin+"."+v.stamp)
}
// livePath is the destination after swap.
// e.g. /configs/plugins/molecule-skill-foo
func (v installVersion) livePath() string {
return path.Join(pluginsRoot, v.plugin)
}
// markerPath is the .complete file inside the live dir written last.
func (v installVersion) markerPath() string {
return path.Join(v.livePath(), completeMarker)
}
// atomicCopyToContainer does a stage→snapshot→swap→marker install of a
// host-side staged plugin tree into a running container's
// /configs/plugins/<name>/. Returns nil on success.
//
// On post-snapshot failure (swap or marker write), best-effort rollback
// restores the previous snapshot to the live path. Returns the original
// error wrapped — the caller should surface it; rollback success is
// logged separately.
func (h *PluginsHandler) atomicCopyToContainer(
ctx context.Context, containerName, hostDir, pluginName string,
) error {
v := newInstallVersion(pluginName)
// Step 0a: ensure staging + previous root dirs exist (idempotent).
if _, err := h.execAsRoot(ctx, containerName, []string{
"mkdir", "-p", pluginsStagingDir, pluginsPrevDir,
}); err != nil {
return fmt.Errorf("atomic install: mkdir staging/previous: %w", err)
}
// Step 0b: tar the host content with a path prefix that lands it in the
// staging dir — NOT directly into the live name. The prefix has no
// leading "/" because docker.CopyToContainer extracts paths relative
// to the dstPath argument we pass below.
stagedRel := strings.TrimPrefix(v.stagedPath(), "/")
tarBuf, err := tarHostDirWithPrefix(hostDir, stagedRel)
if err != nil {
return fmt.Errorf("atomic install: tar host dir: %w", err)
}
// Step 1: STAGE — extract tar into /configs/plugins/.staging/<name>.<ts>/
if err := h.docker.CopyToContainer(ctx, containerName, "/", &tarBuf,
container.CopyToContainerOptions{}); err != nil {
// Best-effort: clean up any partial staging extract before returning.
_, _ = h.execAsRoot(ctx, containerName, []string{
"rm", "-rf", v.stagedPath(),
})
return fmt.Errorf("atomic install: copy to container: %w", err)
}
// Step 2: SNAPSHOT — if a live version exists, move it aside.
// `test -d` exits 0 if the dir exists, non-zero otherwise; the helper
// returns a non-nil error in the non-zero case which we treat as
// "no previous version" rather than a real failure.
snapshotted := false
if _, err := h.execAsRoot(ctx, containerName, []string{
"test", "-d", v.livePath(),
}); err == nil {
if _, err := h.execAsRoot(ctx, containerName, []string{
"mv", v.livePath(), v.previousPath(),
}); err != nil {
// Snapshot failure: roll back the staged extract before failing.
_, _ = h.execAsRoot(ctx, containerName, []string{
"rm", "-rf", v.stagedPath(),
})
return fmt.Errorf("atomic install: snapshot previous version: %w", err)
}
snapshotted = true
}
// Step 3: SWAP — atomic rename of the staged dir into the live name.
// `mv` on the same filesystem is a single rename(2), atomic at the FS level.
if _, err := h.execAsRoot(ctx, containerName, []string{
"mv", v.stagedPath(), v.livePath(),
}); err != nil {
// Swap failure: roll back if we had a snapshot.
if snapshotted {
if _, rbErr := h.execAsRoot(ctx, containerName, []string{
"mv", v.previousPath(), v.livePath(),
}); rbErr != nil {
return fmt.Errorf("atomic install: swap failed AND rollback failed: swap=%w, rollback=%v", err, rbErr)
}
}
// Best-effort cleanup of the still-staged dir.
_, _ = h.execAsRoot(ctx, containerName, []string{
"rm", "-rf", v.stagedPath(),
})
return fmt.Errorf("atomic install: swap to live path: %w", err)
}
// Step 4: MARKER — touch .complete inside the live dir as the last write.
// Workspace-side plugin loaders treat a plugin dir without this marker
// as half-installed and skip it (or surface a clear error to the
// operator instead of loading a possibly-partial tree).
if _, err := h.execAsRoot(ctx, containerName, []string{
"touch", v.markerPath(),
}); err != nil {
// Marker write failure with the new content already in place is a
// weird state — content is fine on disk, but the plugin loader
// will refuse to use it. Log loudly; do NOT roll back, since the
// content is the latest, just unmarked. Operator can manually
// `touch <plugin>/.complete` to recover.
return fmt.Errorf("atomic install: write .complete marker (content landed but unmarked, manual recovery: touch %s): %w", v.markerPath(), err)
}
// Step 5: GC — best-effort delete the previous snapshot. Failures here
// just leave a directory; not load-bearing for correctness, the next
// install or a separate sweeper will reclaim the space.
if snapshotted {
_, _ = h.execAsRoot(ctx, containerName, []string{
"rm", "-rf", v.previousPath(),
})
}
return nil
}
// tarHostDirWithPrefix walks hostDir and writes a tar to a buffer with
// every entry's name prefixed by `prefix`. Mirrors the prior streaming
// shape used in copyPluginToContainer but with a configurable prefix
// (the prior version hardcoded "plugins/<name>/"; we use a full
// staging path so the extracted layout is the staging dir directly).
//
// Symlinks are skipped — same posture as streamDirAsTar elsewhere in
// this file. Skipping prevents a hostile plugin from injecting a
// symlink that, post-extract, points outside the plugin's own dir.
func tarHostDirWithPrefix(hostDir, prefix string) (bytes.Buffer, error) {
var buf bytes.Buffer
tw := newTarWriter(&buf)
defer tw.Close()
if err := tarWalk(hostDir, prefix, tw); err != nil {
return bytes.Buffer{}, err
}
return buf, nil
}
@@ -0,0 +1,70 @@
package handlers
// plugins_atomic_tar.go — tar-walk helpers split out so the main atomic
// install flow stays readable. The prefix argument lets the caller
// arrange where the tar's contents land at extract time.
import (
"archive/tar"
"io"
"os"
"path/filepath"
)
// newTarWriter is a thin wrapper so atomic_test.go can swap the writer
// destination if it needs to.
func newTarWriter(w io.Writer) *tar.Writer {
return tar.NewWriter(w)
}
// tarWalk walks hostDir and writes every regular file + dir to the tar
// writer with paths of the form `<prefix>/<relative>`. Symlinks are
// skipped — same posture as streamDirAsTar in plugins_install_pipeline.go.
//
// The trailing-slash on prefix is normalized away: prefix "foo" and
// prefix "foo/" produce identical archives.
func tarWalk(hostDir, prefix string, tw *tar.Writer) error {
prefix = filepath.Clean(prefix)
return filepath.Walk(hostDir, func(p string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if info.Mode()&os.ModeSymlink != 0 {
return nil // skip symlinks; see doc above
}
rel, err := filepath.Rel(hostDir, p)
if err != nil {
return err
}
if rel == "." {
// Emit the prefix dir itself once, with the source dir's mode.
hdr, err := tar.FileInfoHeader(info, "")
if err != nil {
return err
}
hdr.Name = prefix + "/"
return tw.WriteHeader(hdr)
}
hdr, err := tar.FileInfoHeader(info, "")
if err != nil {
return err
}
hdr.Name = filepath.Join(prefix, rel)
if info.IsDir() {
hdr.Name += "/"
}
if err := tw.WriteHeader(hdr); err != nil {
return err
}
if !info.Mode().IsRegular() {
return nil
}
f, err := os.Open(p)
if err != nil {
return err
}
defer f.Close()
_, err = io.Copy(tw, f)
return err
})
}
@@ -0,0 +1,193 @@
package handlers
import (
"archive/tar"
"bytes"
"io"
"os"
"path/filepath"
"sort"
"strings"
"testing"
"time"
)
// TestInstallVersion_Paths: the path helpers must produce a stable shape
// the in-container exec calls depend on. Pinning the layout here
// catches a future refactor that accidentally changes where staging /
// previous / live dirs live, which would break the swap atomicity.
func TestInstallVersion_Paths(t *testing.T) {
v := installVersion{plugin: "molecule-skill-foo", stamp: "20260508T141530Z"}
if got, want := v.stagedPath(), "/configs/plugins/.staging/molecule-skill-foo.20260508T141530Z"; got != want {
t.Errorf("stagedPath = %q; want %q", got, want)
}
if got, want := v.previousPath(), "/configs/plugins/.previous/molecule-skill-foo.20260508T141530Z"; got != want {
t.Errorf("previousPath = %q; want %q", got, want)
}
if got, want := v.livePath(), "/configs/plugins/molecule-skill-foo"; got != want {
t.Errorf("livePath = %q; want %q", got, want)
}
if got, want := v.markerPath(), "/configs/plugins/molecule-skill-foo/.complete"; got != want {
t.Errorf("markerPath = %q; want %q", got, want)
}
}
// TestInstallVersion_StampUniqueness: two newInstallVersion calls within
// the same second produce the same stamp (we use second precision); the
// caller relies on the mv-rename being atomic, so collision-free
// stamping is NOT a correctness requirement — but a regression that
// changes stamp shape (e.g. RFC3339 with colons) would break the path
// helpers since path.Join treats a colon as a regular char but ssh +
// docker exec generally don't. Pin the no-colon shape.
func TestInstallVersion_StampShape(t *testing.T) {
v := newInstallVersion("anything")
if strings.Contains(v.stamp, ":") {
t.Errorf("stamp must not contain colons (breaks shell-quoting in exec): %q", v.stamp)
}
if strings.Contains(v.stamp, " ") {
t.Errorf("stamp must not contain spaces: %q", v.stamp)
}
// Sanity: stamp parses as the documented format.
if _, err := time.Parse("20060102T150405Z", v.stamp); err != nil {
t.Errorf("stamp %q does not parse as 20060102T150405Z: %v", v.stamp, err)
}
}
// TestTarHostDirWithPrefix_HappyPath: walks a host dir, builds a tar with
// the configured prefix, verifies every entry's name is rooted under
// the prefix, and the file contents survive round-trip.
func TestTarHostDirWithPrefix_HappyPath(t *testing.T) {
hostDir := t.TempDir()
// Plant: <host>/plugin.yaml + <host>/skills/foo/SKILL.md + <host>/.complete
files := map[string]string{
"plugin.yaml": "name: foo\nversion: 1.0.0\n",
"skills/foo/SKILL.md": "# Foo skill\n",
".complete": "", // upstream may already have a marker
}
for rel, body := range files {
full := filepath.Join(hostDir, rel)
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(full, []byte(body), 0o644); err != nil {
t.Fatal(err)
}
}
prefix := "configs/plugins/.staging/foo.20260508T141530Z"
buf, err := tarHostDirWithPrefix(hostDir, prefix)
if err != nil {
t.Fatalf("tar: %v", err)
}
// Read back the tar; collect names + body for regular files.
got := map[string]string{}
tr := tar.NewReader(&buf)
for {
hdr, err := tr.Next()
if err == io.EOF {
break
}
if err != nil {
t.Fatalf("tar reader: %v", err)
}
// Every entry must start with the prefix
if !strings.HasPrefix(hdr.Name, prefix) {
t.Errorf("entry %q does not start with prefix %q", hdr.Name, prefix)
}
if hdr.Typeflag == tar.TypeReg {
body, err := io.ReadAll(tr)
if err != nil {
t.Fatal(err)
}
rel := strings.TrimPrefix(hdr.Name, prefix+"/")
got[rel] = string(body)
}
}
for rel, want := range files {
if got[rel] != want {
t.Errorf("body[%q] = %q; want %q", rel, got[rel], want)
}
}
}
// TestTarHostDirWithPrefix_SkipsSymlinks: a hostile plugin shouldn't be
// able to ship a symlink that, post-extract, points outside its own
// dir. The walker silently skips symlinks (same posture as
// streamDirAsTar). Verify a planted symlink doesn't appear in the tar.
func TestTarHostDirWithPrefix_SkipsSymlinks(t *testing.T) {
hostDir := t.TempDir()
// Plant a real file + a symlink pointing outside hostDir.
if err := os.WriteFile(filepath.Join(hostDir, "real.txt"), []byte("ok"), 0o644); err != nil {
t.Fatal(err)
}
target := filepath.Join(t.TempDir(), "outside")
if err := os.WriteFile(target, []byte("SHOULD NOT APPEAR"), 0o644); err != nil {
t.Fatal(err)
}
if err := os.Symlink(target, filepath.Join(hostDir, "evil")); err != nil {
t.Fatal(err)
}
buf, err := tarHostDirWithPrefix(hostDir, "p")
if err != nil {
t.Fatal(err)
}
names := []string{}
tr := tar.NewReader(&buf)
for {
hdr, err := tr.Next()
if err == io.EOF {
break
}
if err != nil {
t.Fatal(err)
}
names = append(names, hdr.Name)
}
sort.Strings(names)
for _, n := range names {
if strings.Contains(n, "evil") {
t.Errorf("symlink leaked into tar: %q", n)
}
}
// real.txt should be present
found := false
for _, n := range names {
if strings.HasSuffix(n, "real.txt") {
found = true
break
}
}
if !found {
t.Errorf("real.txt missing from tar; got names: %v", names)
}
}
// TestTarHostDirWithPrefix_PrefixNormalization: trailing slash on prefix
// should not change the archive shape. Pinning this so a future caller
// passing "foo/" instead of "foo" doesn't double-slash entry names.
func TestTarHostDirWithPrefix_PrefixNormalization(t *testing.T) {
hostDir := t.TempDir()
if err := os.WriteFile(filepath.Join(hostDir, "x"), []byte("y"), 0o644); err != nil {
t.Fatal(err)
}
a, err := tarHostDirWithPrefix(hostDir, "foo")
if err != nil {
t.Fatal(err)
}
b, err := tarHostDirWithPrefix(hostDir, "foo/")
if err != nil {
t.Fatal(err)
}
if !bytes.Equal(a.Bytes(), b.Bytes()) {
t.Errorf("trailing-slash on prefix changed archive shape; tarHostDirWithPrefix should be slash-insensitive")
}
}
@@ -276,7 +276,13 @@ func (h *PluginsHandler) resolveAndStage(ctx context.Context, req installRequest
// using NewPluginsHandler without a DB; production wires it in router.go.
func (h *PluginsHandler) deliverToContainer(ctx context.Context, workspaceID string, r *stageResult) error {
if containerName := h.findRunningContainer(ctx, workspaceID); containerName != "" {
if err := h.copyPluginToContainer(ctx, containerName, r.StagedDir, r.PluginName); err != nil {
// Atomic stage→snapshot→swap→marker (molecule-core#114).
// Replaces the prior single docker.CopyToContainer write that
// left a partially-extracted tree on mid-install failure with
// no rollback path. atomicCopyToContainer writes a .complete
// marker as the last step; workspace-side plugin loaders should
// refuse to load a plugin dir without it.
if err := h.atomicCopyToContainer(ctx, containerName, r.StagedDir, r.PluginName); err != nil {
log.Printf("Plugin install: failed to copy %s to %s: %v", r.PluginName, workspaceID, err)
return newHTTPErr(http.StatusInternalServerError, gin.H{"error": "failed to copy plugin to container"})
}