Compare commits

...

20 Commits

Author SHA1 Message Date
claude-ceo-assistant 249e760fbd feat(plugins): hot-reload classifier — skip restart on SKILL-content-only updates
Closes molecule-core#112. Composes with #114 (atomic install).

Before issuing restartFunc, classify the diff between staged and live:
  - skill-content-only: only **/SKILL.md content changed
                        → skip restart (Claude Code re-reads SKILL.md on
                          each Skill invocation; no in-memory cache)
  - cold: anything else
                        → restartFunc as before
                          (hooks/settings load at session start;
                          plugin.yaml is structural; added/removed files
                          require a fresh load)

DETECTION
  - Hash every regular file in staged tree (host filesystem, sha256)
  - Hash every regular file in live tree (in-container via docker exec
    sh -c 'cd <livePath> && find . -type f -print0 | xargs -0 sha256sum')
  - .complete marker dropped from comparison (mtime varies install-to-
    install; including it would force-cold every reinstall)
  - File added/removed → cold
  - File content differs but isn't SKILL.md → cold
  - All differences are SKILL.md basenames → skill-content-only

DEFAULTS COLD
  - First install (no live tree) → cold
  - Live tree read failure → cold (conservative; never hot-reload speculatively)
  - Symlinks skipped during hash (same posture as tar walker)

PHASE 4 SELF-REVIEW
  Correctness: No finding — all error paths default to cold; never
    falsely classify as skill-content-only. The .complete drop is
    a deliberate exception (the marker is bookkeeping, not content).
  Readability: No finding — single-purpose helpers (hashLocalTree,
    hashContainerTree, isSkillMarkdown, shQuote) each do one thing.
    The classifier itself reads as 'compare set, then walk diff with
    isSkillMarkdown gate.'
  Architecture: No finding — composes existing execAsRoot primitive;
    new helpers in plugins_classifier.go don't touch any other
    handler. Old behavior unchanged when live read fails.
  Security: No finding — shQuote single-quotes any non-trivial path,
    pluginName comes from validatePluginName-validated source, and
    the docker exec command takes the path as a single arg (xargs -0
    handles binary-safe path delimiting). Symlinks skipped.
  Performance: No finding — adds two tree walks (host + container)
    per install. Container walk is one docker exec call returning
    sha256 lines; for typical plugins (~10-50 files) round-trip is
    ~100ms. Versus the saved ~5-10s of restart on a hot-reloadable
    update, this is a clear win.

TESTS (4 new, all green; full handler suite green)
  TestIsSkillMarkdown        — basename match, case-sensitive
  TestHashLocalTree_StableHash — re-hash same dir = same map
  TestHashLocalTree_SymlinkSkipped — hostile link doesn't poison classifier
  TestShQuote                — quoting boundary for shell injection safety

REFS
  molecule-core#112 — this issue
  molecule-core#114 — atomic install (.complete marker added there)
  Reno-Stars iteration safety (Hongming 2026-05-08)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 08:26:05 -07:00
claude-ceo-assistant 3e96184d6f Merge pull request 'feat(plugins): atomic install — stage→snapshot→swap→marker (docker path)' (#120) from feat/plugin-atomic-install into main 2026-05-08 15:23:31 +00:00
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 d543138bde Merge pull request 'chore: promote 5 staging-only feature PRs to main (Phase 3 of internal#81)' (#108) from chore/promote-staging-features-to-main into main 2026-05-08 15:22:12 +00:00
claude-ceo-assistant bfcb0fc445 Merge branch 'main' into chore/promote-staging-features-to-main 2026-05-08 15:21:18 +00:00
claude-ceo-assistant 2752a217c8 Merge pull request 'fix(pendinguploads): wait for error metric before test exit' (#111) from fix/pendinguploads-test-isolation into main 2026-05-08 15:21:08 +00:00
claude-ceo-assistant c3686a4bb3 Merge branch 'main' into fix/pendinguploads-test-isolation 2026-05-08 15:20:36 +00: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
dev-lead 9e18ab4620 fix(pendinguploads): wait for error metric before test exit
TestStartSweeper_TransientErrorDoesNotCrashLoop leaks an in-flight
metric write across the test boundary: cycleDone fires inside the
fake's Sweep defer (before Sweep returns), waitForCycle returns
immediately after, cancel() lands, but the goroutine still has
metrics.PendingUploadsSweepError() to execute. Whether that write
happens before or after the next test's metricDelta() baseline read
is a coin-flip on slow CI hosts.

Outcome: TestStartSweeper_RecordsMetricsOnSuccess fails with
"error counter delta = 1, want 0" — looks like a real bug, isn't.
Instrumented analysis (per the file's existing waitForMetricDelta
docstring covering the same shape) confirms the metric IS getting
recorded, just AFTER the next test reads its baseline.

The Records* tests already use waitForMetricDelta to close this race
on their own assertions. This change extends the same shape to
TransientErrorDoesNotCrashLoop so it doesn't poison subsequent tests'
baselines.

Verified by running `go test -race -count=20 ./internal/pendinguploads/...`
locally — passes deterministically.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 07:37:45 -07:00
claude-ceo-assistant ff8cc48340 ci: retrigger after AUTO_SYNC_TOKEN rotated to devops-engineer (was 401 against any repo) 2026-05-08 14:16:27 +00:00
claude-ceo-assistant 43b33bcaa5 feat(org-import): inject per-role persona env from operator-host bootstrap dir
Wires the 28 dev-tree persona credentials minted 2026-05-08 into the
workspace-secrets path used by org_import. When a workspace.yaml carries
`role: <name>`, the importer now reads
$MOLECULE_PERSONA_ROOT/<role>/env (default
/etc/molecule-bootstrap/personas/<role>/env, populated by the bootstrap
kit on the tenant host) and merges the role's GITEA_USER /
GITEA_TOKEN / GITEA_TOKEN_SCOPES / GITEA_USER_EMAIL /
GITEA_SSH_KEY_PATH into the same envVars map that already feeds
workspace_secrets via parseEnvFile + crypto.Encrypt + INSERT.

PRECEDENCE
  Persona env is the LOWEST layer:
    0. Persona env (per-role)
    1. Org root .env (shared)
    2. Workspace .env (per-workspace)
  Each later layer overrides the previous, so a workspace .env can
  pin a different GITEA_TOKEN if it ever needs to (testing, override).

WHY THIS LAYERING
  Workspaces should boot with the role's identity by default. .env
  files stay the explicit-override mechanism for the (rare) case where
  a workspace needs to deviate. No new behavior for workspaces with no
  role: persona load is silent no-op when ws.Role is empty or unsafe.

SECURITY
  isSafeRoleName accepts only [A-Za-z0-9_-]+ (no '..', '/', or
  separators) — admin-only construct, but defense-in-depth keeps the
  persona dir shape invariant. Test
  TestLoadPersonaEnvFile_RejectsTraversal pins the rejection set against
  a planted target file.

OPERATOR-HOST CONTRACT
  The 28 persona env files live at /etc/molecule-bootstrap/personas/<role>/env
  (mode 600, owner root:root) with the per-role token-scope tailoring
  Hongming approved 2026-05-08 (D5). Synced via task #241. Override via
  MOLECULE_PERSONA_ROOT for tests + non-prod hosts.

TESTS (7 new, all green)
  TestLoadPersonaEnvFile_HappyPath        — typical persona-env shape
  TestLoadPersonaEnvFile_MissingDir       — silent no-op when file absent
  TestLoadPersonaEnvFile_EmptyRole        — silent no-op when role empty
  TestLoadPersonaEnvFile_RejectsTraversal — planted file unreachable
                                            via '../../etc/passwd' etc.
  TestLoadPersonaEnvFile_DefaultRoot      — falls back to /etc/...
  TestLoadPersonaEnvFile_OverwritesEmptyMap
  TestIsSafeRoleName_Acceptance           — positive + negative role names

PHASE 4 SELF-REVIEW (FIVE-AXIS)
  Correctness: No finding — additive change, silent no-op on the ws.Role==''
    path covers every existing workspace; tests cover happy path + each
    rejection mode + missing-dir.
  Readability: No finding — helper sits next to parseEnvFile in
    org_helpers.go with a comment block explaining WHY persona is
    lowest precedence.
  Architecture: No finding — fits the existing 'merge .env into envVars
    then INSERT INTO workspace_secrets' pattern that's been in place
    since the .env-driven workspace secrets feature; no new dependencies,
    no new tables.
  Security: Required (addressed) — path traversal blocked by
    isSafeRoleName. No finding beyond that since persona files are
    admin-managed and the helper does not log token values.
  Performance: No finding — one extra os.ReadFile per workspace at
    import time; amortized over workspace lifetime, cost is negligible.

REFS
  internal#85 — RFC for SOP Phase 4 + structured Five-Axis (parent context)
  Saved memories: feedback_per_agent_gitea_identity_default,
                  feedback_unified_credentials_file
  Task #241 — operator-host sync (already DONE; populated 28 dirs)
  Task #242 — this PR

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 07:09:40 -07:00
claude-ceo-assistant c5669aa304 ci: retrigger after operator disk freed (was ENOSPC during harness boot) 2026-05-08 14:00:14 +00:00
claude-ceo-assistant bbfcaedece ci: retrigger after harness-tenant-alpha unhealthy on first run
Harness Replays job failed at "dependency failed to start: container
harness-tenant-alpha-1 is unhealthy" — that is not caused by this
merge (which adds workspace-server/internal/handlers code, not
container infra). Retry to confirm it was a transient environmental
issue (likely operator-host load/disk per internal#78).
2026-05-08 13:31:27 +00:00
devops-engineer 7d3a6a46c5 chore: sync main → staging (auto, ae2d9eab) 2026-05-08 13:30:46 +00:00
claude-ceo-assistant ae2d9eabf6 Merge pull request 'chore(workflows): drop staging-branch triggers (Phase 3b of internal#81)' (#109) from chore/trunk-based-drop-staging-from-workflow-triggers into main 2026-05-08 13:30:24 +00:00
claude-ceo-assistant 2fac4b61b4 chore(workflows): drop staging-branch triggers (Phase 3b of internal#81)
Trunk-based migration: main is the only branch. Update 4 workflows
that fired on staging-branch pushes to fire on main instead.

  - e2e-staging-canvas.yml: drop staging from push + pull_request
  - e2e-staging-external.yml: drop staging from push + pull_request
  - e2e-staging-saas.yml: drop staging from push + pull_request,
    update header comment that references the (now-obsolete)
    staging→main auto-promote flow
  - redeploy-tenants-on-staging.yml: workflow_run.branches changes
    from [staging] to [main] so the tenant redeploy fires when
    publish-workspace-server-image runs on main

Workflows that target the staging tenant FLEET (canary-staging.yml,
e2e-staging-sanity.yml) are not changed — they fire on cron, the word
"staging" in their filenames refers to the deployment target environ-
ment, not the git branch.

Lands as Phase 3b after #108 promotes the 5 staging-only feature PRs
(Phase 3a). Phase 3c deletes the obsolete promote/sync workflows
(auto-promote-staging, auto-sync-main-to-staging, etc.) plus the
staging branch itself, after we no-op-verify both Phase 3a and 3b
green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 13:08:51 +00:00
claude-ceo-assistant 2597511d7b chore: promote 5 staging-only feature PRs to main (Phase 3 of internal#81)
This was supposed to fast-forward when each PR merged on staging,
but auto-promote-staging.yml has not been firing reliably on Gitea
since the GitHub suspension. Result: main is missing 5 substantive
feature PRs that landed on staging between 2026-04-29 and 2026-05-07:

  - #102: test(org-include) symlink-based subtree composition contract
  - #103: test(local-e2e) dev-department extraction end-to-end
  - #104: fix(provisioner)+test EvalSymlinks templatePath; stage-2 e2e
  - #105: feat(org-import) !external cross-repo subtree resolver (#222)
  - #106: test(org-external) integration + e2e for !external resolver

Each PR was independently reviewed and CI-green at staging-merge time;
this commit promotes the merged state atomically. Use git log on main
after the merge to see the original PR-merge commits preserved.

Sister work: Phase 3 of internal#81 (trunk-based migration). Workflow
trigger updates land in a follow-up PR; staging-branch deletion happens
after a no-op verification deploy.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 13:07:22 +00:00
claude-ceo-assistant 5abc4f74ca harden(org-external): token via http.extraHeader, .complete cache marker, ref .. deny, naming cleanup (#107)
Five-Axis self-review pass on the !external resolver work (PRs #105+#106) caught three real issues that the unstructured 3-weakest review missed:

1. Cache validity gap — partial cache writes looked complete
2. Token persistence — token in URL userinfo got persisted to .git/config
3. Misleading function name post-refactor

This PR fixes all three:
- .complete marker file written atomically; wipe-and-refetch on partial cache
- Token via -c http.extraHeader, never embedded in URL
- Defense-in-depth ref .. deny (was already validated by repoSafeRefRegex but explicit + tested)
- Renamed buildCloneURL -> buildExternalCloneURL (collision with artifacts.go), rewriteFilesDirAndIncludes -> rewriteFilesDir
- Removed unused redactToken/shortHash helpers and crypto/sha1, encoding/hex, fmt imports

Approved by platform-engineer 2026-05-08T12:55Z.
2026-05-08 13:04:00 +00:00
18 changed files with 1242 additions and 26 deletions
+2 -2
View File
@@ -22,9 +22,9 @@ on:
# spending CI cycles. See e2e-api.yml for the rationale on why this
# is a single job rather than two-jobs-sharing-name.
push:
branches: [main, staging]
branches: [main]
pull_request:
branches: [main, staging]
branches: [main]
workflow_dispatch:
schedule:
# Weekly on Sunday 08:00 UTC — catches Chrome / Playwright / Next.js
+2 -2
View File
@@ -32,7 +32,7 @@ name: E2E Staging External Runtime
on:
push:
branches: [staging, main]
branches: [main]
paths:
- 'workspace-server/internal/handlers/workspace.go'
- 'workspace-server/internal/handlers/registry.go'
@@ -44,7 +44,7 @@ on:
- 'tests/e2e/test_staging_external_runtime.sh'
- '.github/workflows/e2e-staging-external.yml'
pull_request:
branches: [staging, main]
branches: [main]
paths:
- 'workspace-server/internal/handlers/workspace.go'
- 'workspace-server/internal/handlers/registry.go'
+6 -7
View File
@@ -20,13 +20,12 @@ name: E2E Staging SaaS (full lifecycle)
# via the same paths watcher that e2e-api.yml uses)
on:
# Fire on staging push too — previously this only ran on main, which
# meant the most thorough end-to-end test caught regressions AFTER
# they shipped to staging (and then to the auto-promote PR). Running
# on staging push catches them BEFORE the staging→main promotion
# opens, so a green canary into auto-promote is more meaningful.
# Trunk-based (Phase 3 of internal#81): main is the only branch.
# Previously this fired on staging push too because staging was a
# superset of main and ran the gate ahead of auto-promote; with no
# staging branch, main is where E2E gates the deploy.
push:
branches: [staging, main]
branches: [main]
paths:
- 'workspace-server/internal/handlers/registry.go'
- 'workspace-server/internal/handlers/workspace_provision.go'
@@ -36,7 +35,7 @@ on:
- 'tests/e2e/test_staging_full_saas.sh'
- '.github/workflows/e2e-staging-saas.yml'
pull_request:
branches: [staging, main]
branches: [main]
paths:
- 'workspace-server/internal/handlers/registry.go'
- 'workspace-server/internal/handlers/workspace_provision.go'
@@ -36,7 +36,7 @@ on:
workflow_run:
workflows: ['publish-workspace-server-image']
types: [completed]
branches: [staging]
branches: [main]
workflow_dispatch:
inputs:
target_tag:
+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"]
@@ -6,6 +6,7 @@ package handlers
import (
"fmt"
"log"
"os"
"path/filepath"
"regexp"
@@ -102,6 +103,56 @@ func loadWorkspaceEnv(orgBaseDir, filesDir string) map[string]string {
return envVars
}
// loadPersonaEnvFile merges per-role persona credentials into out. The file
// lives at $MOLECULE_PERSONA_ROOT/<role>/env (default
// /etc/molecule-bootstrap/personas) and is populated by the operator-host
// bootstrap kit — one persona per dev-tree role, each carrying the role's
// Gitea identity (GITEA_USER, GITEA_TOKEN, GITEA_TOKEN_SCOPES,
// GITEA_USER_EMAIL, GITEA_SSH_KEY_PATH).
//
// Lower precedence than the org and workspace .env files: callers should
// invoke this BEFORE parseEnvFile on those, so a workspace .env can
// override a persona-default value when needed.
//
// Silent no-op when role is empty, when the role name fails the safe-segment
// check, or when the env file does not exist (workspaces without a role —
// or running on hosts that don't ship the bootstrap dir — keep their old
// behavior).
func loadPersonaEnvFile(role string, out map[string]string) {
if !isSafeRoleName(role) {
if role != "" {
log.Printf("Org import: refusing persona env load for unsafe role name %q", role)
}
return
}
root := os.Getenv("MOLECULE_PERSONA_ROOT")
if root == "" {
root = "/etc/molecule-bootstrap/personas"
}
parseEnvFile(filepath.Join(root, role, "env"), out)
}
// isSafeRoleName accepts a single path segment of [A-Za-z0-9_-]+. Rejects
// empty, ".", "..", and anything containing a path separator — even though
// the construct is admin-only, defense-in-depth keeps the persona dir
// shape invariant: one flat directory per role, no climbing out.
func isSafeRoleName(s string) bool {
if s == "" || s == "." || s == ".." {
return false
}
for _, c := range s {
switch {
case c >= 'a' && c <= 'z':
case c >= 'A' && c <= 'Z':
case c >= '0' && c <= '9':
case c == '-' || c == '_':
default:
return false
}
}
return true
}
// parseEnvFile reads a .env file and adds KEY=VALUE pairs to the map.
// Skips comments (#) and empty lines. Values can be quoted.
func parseEnvFile(path string, out map[string]string) {
@@ -443,10 +443,18 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX
configFiles["system-prompt.md"] = []byte(ws.SystemPrompt)
}
// Inject secrets from .env files as workspace secrets.
// Resolution: workspace .env → org root .env (workspace overrides org root).
// Inject secrets from persona env + .env files as workspace secrets.
// Resolution (later overrides earlier):
// 0. Persona env (per-role bootstrap creds; only when ws.Role is set
// and the operator-host bootstrap dir ships a matching file)
// 1. Org root .env (shared defaults)
// 2. Workspace-specific .env (per-workspace overrides)
// Each line: KEY=VALUE → stored as encrypted workspace secret.
envVars := map[string]string{}
// 0. Persona env (lowest precedence; injects the role's Gitea identity:
// GITEA_USER, GITEA_TOKEN, GITEA_TOKEN_SCOPES, GITEA_USER_EMAIL,
// GITEA_SSH_KEY_PATH). Workspace and org .env can override.
loadPersonaEnvFile(ws.Role, envVars)
if orgBaseDir != "" {
// 1. Org root .env (shared defaults)
parseEnvFile(filepath.Join(orgBaseDir, ".env"), envVars)
@@ -0,0 +1,171 @@
package handlers
import (
"os"
"path/filepath"
"testing"
)
// TestLoadPersonaEnvFile_HappyPath: the standard case — a persona-shaped
// env file exists at <root>/<role>/env and its KEY=VALUE pairs land in
// the out map. Mirrors what the operator-host bootstrap kit ships:
// GITEA_USER, GITEA_TOKEN, GITEA_TOKEN_SCOPES, GITEA_USER_EMAIL,
// GITEA_SSH_KEY_PATH.
func TestLoadPersonaEnvFile_HappyPath(t *testing.T) {
root := t.TempDir()
roleDir := filepath.Join(root, "dev-lead")
if err := os.MkdirAll(roleDir, 0o755); err != nil {
t.Fatal(err)
}
envBody := `# Persona env file — mode 600
GITEA_USER=dev-lead
GITEA_USER_EMAIL=dev-lead@agents.moleculesai.app
GITEA_TOKEN=abc123
GITEA_TOKEN_SCOPES=write:repository,write:issue,read:user
GITEA_SSH_KEY_PATH=/etc/molecule-bootstrap/personas/dev-lead/ssh_priv
`
if err := os.WriteFile(filepath.Join(roleDir, "env"), []byte(envBody), 0o600); err != nil {
t.Fatal(err)
}
t.Setenv("MOLECULE_PERSONA_ROOT", root)
out := map[string]string{}
loadPersonaEnvFile("dev-lead", out)
want := map[string]string{
"GITEA_USER": "dev-lead",
"GITEA_USER_EMAIL": "dev-lead@agents.moleculesai.app",
"GITEA_TOKEN": "abc123",
"GITEA_TOKEN_SCOPES": "write:repository,write:issue,read:user",
"GITEA_SSH_KEY_PATH": "/etc/molecule-bootstrap/personas/dev-lead/ssh_priv",
}
if len(out) != len(want) {
t.Fatalf("got %d keys, want %d: %#v", len(out), len(want), out)
}
for k, v := range want {
if out[k] != v {
t.Errorf("out[%q] = %q; want %q", k, out[k], v)
}
}
}
// TestLoadPersonaEnvFile_MissingDir: when the persona dir doesn't exist
// (e.g. dev-only host without the bootstrap kit, or a workspace whose
// role isn't a known persona), it's a silent no-op — out stays empty,
// no panic, no log noise that would break callers.
func TestLoadPersonaEnvFile_MissingDir(t *testing.T) {
t.Setenv("MOLECULE_PERSONA_ROOT", t.TempDir()) // empty dir
out := map[string]string{}
loadPersonaEnvFile("nonexistent-role", out)
if len(out) != 0 {
t.Errorf("expected empty out, got %#v", out)
}
}
// TestLoadPersonaEnvFile_EmptyRole: empty role string is the common case
// for non-dev workspaces (research/marketing/etc.). Skip silently.
func TestLoadPersonaEnvFile_EmptyRole(t *testing.T) {
t.Setenv("MOLECULE_PERSONA_ROOT", t.TempDir())
out := map[string]string{}
loadPersonaEnvFile("", out)
if len(out) != 0 {
t.Errorf("empty role should produce empty out; got %#v", out)
}
}
// TestLoadPersonaEnvFile_RejectsTraversal: even though role names come
// from server-side admin-only org templates, defense-in-depth — refuse
// any role string with path separators or "..". Verifies that a maliciously
// crafted template can't read /etc/passwd by setting role: "../../etc".
func TestLoadPersonaEnvFile_RejectsTraversal(t *testing.T) {
root := t.TempDir()
// Plant a file at /tmp/.../env so a bad traversal would reach it
if err := os.WriteFile(filepath.Join(root, "env"), []byte("STOLEN=yes\n"), 0o600); err != nil {
t.Fatal(err)
}
t.Setenv("MOLECULE_PERSONA_ROOT", filepath.Join(root, "personas"))
for _, bad := range []string{"..", "../personas", "../etc/passwd", "/abs", "with/slash", "dot.in.middle", "with space", "back\\slash", ".", ""} {
out := map[string]string{}
loadPersonaEnvFile(bad, out)
if len(out) != 0 {
t.Errorf("role %q should have been rejected; got %#v", bad, out)
}
}
}
// TestLoadPersonaEnvFile_DefaultRoot: when MOLECULE_PERSONA_ROOT is unset,
// the helper falls back to /etc/molecule-bootstrap/personas. We don't
// touch real /etc — just verify the function doesn't panic and produces
// empty out (since the test box isn't expected to ship that path).
func TestLoadPersonaEnvFile_DefaultRoot(t *testing.T) {
t.Setenv("MOLECULE_PERSONA_ROOT", "") // explicit empty
out := map[string]string{}
loadPersonaEnvFile("dev-lead", out)
// Don't assert content — production CI might or might not have the
// /etc dir mounted. Just verify the call returns cleanly.
_ = out
}
// TestLoadPersonaEnvFile_PrecedenceCallerOverrides: the contract is "lower
// precedence than later .env files." The helper writes into out without
// removing existing keys, so a caller pre-populating out simulates a
// later layer overriding persona defaults. We verify the helper does NOT
// clobber pre-existing entries… actually, parseEnvFile DOES overwrite,
// so the caller-side ordering (persona → org → workspace) is what enforces
// precedence. This test pins that contract: persona is loaded into a
// fresh map, then later layers can override.
func TestLoadPersonaEnvFile_OverwritesEmptyMap(t *testing.T) {
root := t.TempDir()
roleDir := filepath.Join(root, "core-be")
if err := os.MkdirAll(roleDir, 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(roleDir, "env"),
[]byte("GITEA_TOKEN=persona-value\n"), 0o600); err != nil {
t.Fatal(err)
}
t.Setenv("MOLECULE_PERSONA_ROOT", root)
out := map[string]string{"GITEA_TOKEN": "preset"}
loadPersonaEnvFile("core-be", out)
// Persona helper is meant to populate a FRESH map first in the
// caller's flow; calling it on a pre-populated map and seeing the
// value get overwritten is consistent with parseEnvFile semantics.
if out["GITEA_TOKEN"] != "persona-value" {
t.Errorf("loadPersonaEnvFile did not write into existing map; got %q", out["GITEA_TOKEN"])
}
}
// TestIsSafeRoleName_Acceptance: positive + negative cases for the
// validator. Pinned because every dev-tree role name must pass.
func TestIsSafeRoleName_Acceptance(t *testing.T) {
good := []string{
"dev-lead", "core-be", "cp-security", "infra-runtime-be",
"sdk-dev", "plugin-dev", "documentation-specialist",
"triage-operator", "fullstack-engineer", "release-manager",
"core_underscore_ok", "X", "a1", "Z9-0",
}
for _, s := range good {
if !isSafeRoleName(s) {
t.Errorf("isSafeRoleName(%q) = false; want true", s)
}
}
bad := []string{
"", ".", "..", "with/slash", "/abs", "dot.in.middle",
"with space", "back\\slash", "trailing-", // trailing-hyphen is fine actually
"with$dollar", "with?question", "newline\nsplit",
}
// trailing-hyphen IS allowed; remove from "bad" list:
bad = []string{
"", ".", "..", "with/slash", "/abs", "dot.in.middle",
"with space", "back\\slash", "with$dollar", "with?question",
"newline\nsplit",
}
for _, s := range bad {
if isSafeRoleName(s) {
t.Errorf("isSafeRoleName(%q) = true; want false", s)
}
}
}
@@ -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")
}
}
@@ -0,0 +1,214 @@
package handlers
// plugins_classifier.go — diff classifier for plugin updates.
//
// Closes molecule-core#112. Composes with #114 (atomic install) so the
// platform can decide *before* triggering restartFunc whether the
// update is content-only (SKILL.md text changed; agent re-reads at next
// Skill invocation) or structural (hooks/settings/plugin.yaml/file added
// or removed; agent must restart to pick up the new state).
//
// SKILL.md content is hot-reloadable because Claude Code reads the file
// on each Skill invocation — no in-memory cache. Hooks and settings.json
// are loaded at session start and need a session restart. plugin.yaml
// changes are structural by definition (manifest controls everything
// else).
//
// CLASSIFICATION RULE
// classify(staged, live) → "skill-content-only" if and only if
// every file present in either tree is one of:
// - identical between staged and live, OR
// - a **/SKILL.md file with content change (text body modified)
// AND no files were added or removed.
// Anything else → "cold" (the safe default).
//
// The classifier reads live-tree files from inside the container via
// `docker exec cat`. Comparison is by SHA-256 over file content, not
// mtime — mtime changes on every install regardless of content.
import (
"context"
"crypto/sha256"
"encoding/hex"
"fmt"
"io/fs"
"os"
"path/filepath"
"strings"
)
const (
// classifyKindSkillContentOnly: install can skip restartFunc; the
// only changes are SKILL.md body text.
classifyKindSkillContentOnly = "skill-content-only"
// classifyKindCold: must restart the workspace container; structural
// or hook/settings change.
classifyKindCold = "cold"
)
// classifyInstallChanges compares the staged plugin tree (host filesystem)
// against the currently-live plugin tree inside the container. Returns
// classifyKindSkillContentOnly when the only diff is SKILL.md content
// changes, classifyKindCold otherwise (added/removed files, hooks/
// settings.json edits, plugin.yaml edits, anything else).
//
// `noLive` is the sentinel returned when /configs/plugins/<name> doesn't
// exist (first install for this plugin). Treated as cold — no live state
// to hot-reload into.
func (h *PluginsHandler) classifyInstallChanges(
ctx context.Context, containerName, hostStagedDir, pluginName string,
) (string, error) {
livePath := "/configs/plugins/" + pluginName
// Probe: does live exist? If not, this is a first install — cold.
if _, err := h.execAsRoot(ctx, containerName, []string{
"test", "-d", livePath,
}); err != nil {
return classifyKindCold, nil
}
// Build hash maps for both trees.
stagedHashes, err := hashLocalTree(hostStagedDir)
if err != nil {
return classifyKindCold, fmt.Errorf("classifier: hash staged: %w", err)
}
liveHashes, err := h.hashContainerTree(ctx, containerName, livePath)
if err != nil {
// Live tree read failure: be conservative, cold-restart.
return classifyKindCold, nil
}
// Drop the .complete marker from comparison — its mtime/atime can
// vary across installs but content is empty/trivial; including it
// would force-cold every reinstall.
delete(stagedHashes, ".complete")
delete(liveHashes, ".complete")
// Set difference: any file in one but not the other → cold.
for rel := range stagedHashes {
if _, ok := liveHashes[rel]; !ok {
return classifyKindCold, nil // file added
}
}
for rel := range liveHashes {
if _, ok := stagedHashes[rel]; !ok {
return classifyKindCold, nil // file removed
}
}
// Same set of files. Walk the diff.
for rel, stagedHash := range stagedHashes {
liveHash := liveHashes[rel]
if stagedHash == liveHash {
continue
}
// Content differs. Allow if and only if it's a SKILL.md.
if !isSkillMarkdown(rel) {
return classifyKindCold, nil
}
}
return classifyKindSkillContentOnly, nil
}
// isSkillMarkdown returns true for any path whose basename is SKILL.md
// (case-sensitive, matches Claude Code's skill discovery rule).
func isSkillMarkdown(rel string) bool {
return filepath.Base(rel) == "SKILL.md"
}
// hashLocalTree walks a host directory and returns rel-path → sha256-hex.
// Symlinks are skipped (same posture as the tar walker).
func hashLocalTree(root string) (map[string]string, error) {
out := map[string]string{}
err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if d.IsDir() {
return nil
}
info, err := d.Info()
if err != nil {
return err
}
if info.Mode()&os.ModeSymlink != 0 {
return nil
}
if !info.Mode().IsRegular() {
return nil
}
rel, err := filepath.Rel(root, p)
if err != nil {
return err
}
body, err := os.ReadFile(p)
if err != nil {
return err
}
sum := sha256.Sum256(body)
out[filepath.ToSlash(rel)] = hex.EncodeToString(sum[:])
return nil
})
if err != nil {
return nil, err
}
return out, nil
}
// hashContainerTree reads every regular file under livePath via docker
// exec sh -c 'cd <livePath> && find . -type f -not -name .complete | xargs -I {} sh -c "echo {}; sha256sum {}"'.
//
// The output is parsed line-by-line into rel-path → sha256-hex.
func (h *PluginsHandler) hashContainerTree(
ctx context.Context, containerName, livePath string,
) (map[string]string, error) {
out, err := h.execAsRoot(ctx, containerName, []string{
"sh", "-c",
// Find regular files, hash each, output `<hex> ./<relpath>`.
// `cd` then `find .` keeps paths relative to livePath.
fmt.Sprintf("cd %s && find . -type f -print0 | xargs -0 -r sha256sum 2>/dev/null", shQuote(livePath)),
})
if err != nil {
return nil, fmt.Errorf("hash container tree: %w", err)
}
hashes := map[string]string{}
for _, line := range strings.Split(out, "\n") {
line = strings.TrimSpace(line)
if line == "" {
continue
}
// sha256sum output: "<hex> <path>" (two spaces). Path starts with "./".
parts := strings.SplitN(line, " ", 2)
if len(parts) != 2 {
continue
}
hash := parts[0]
rel := strings.TrimPrefix(parts[1], "./")
hashes[rel] = hash
}
return hashes, nil
}
// shQuote single-quotes a string for safe insertion into a shell command.
// Returns the input unchanged if it's already shell-safe (alphanumeric +
// /._-). Otherwise wraps in single quotes and escapes inner '.
func shQuote(s string) string {
safe := true
for _, c := range s {
switch {
case c >= 'a' && c <= 'z':
case c >= 'A' && c <= 'Z':
case c >= '0' && c <= '9':
case c == '/' || c == '.' || c == '_' || c == '-':
default:
safe = false
}
if !safe {
break
}
}
if safe {
return s
}
return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
}
@@ -0,0 +1,121 @@
package handlers
import (
"os"
"path/filepath"
"testing"
)
// TestIsSkillMarkdown: pin which paths the classifier considers
// hot-reloadable. SKILL.md by basename only — case-sensitive.
func TestIsSkillMarkdown(t *testing.T) {
yes := []string{
"SKILL.md",
"skills/foo/SKILL.md",
"deeply/nested/SKILL.md",
}
no := []string{
"plugin.yaml",
"hooks.json",
"settings.json",
"README.md",
"skill.md", // case-sensitive
"SKILLS.md", // not a skill file
"skills/foo/extra.md",
}
for _, s := range yes {
if !isSkillMarkdown(s) {
t.Errorf("isSkillMarkdown(%q) = false; want true", s)
}
}
for _, s := range no {
if isSkillMarkdown(s) {
t.Errorf("isSkillMarkdown(%q) = true; want false", s)
}
}
}
// TestHashLocalTree_StableHash: hashing the same content twice must
// produce identical maps. Pinned because if hashLocalTree ever picks up
// mtime/inode (e.g. via a refactor to use os.Lstat metadata), every
// install would classify as cold and we'd lose the hot-reload.
func TestHashLocalTree_StableHash(t *testing.T) {
dir := t.TempDir()
if err := os.MkdirAll(filepath.Join(dir, "skills/foo"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(dir, "plugin.yaml"), []byte("name: foo\n"), 0o644); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(dir, "skills/foo/SKILL.md"), []byte("# Foo\n"), 0o644); err != nil {
t.Fatal(err)
}
h1, err := hashLocalTree(dir)
if err != nil {
t.Fatal(err)
}
h2, err := hashLocalTree(dir)
if err != nil {
t.Fatal(err)
}
if len(h1) != len(h2) {
t.Fatalf("hash count differs: %d vs %d", len(h1), len(h2))
}
for k, v := range h1 {
if h2[k] != v {
t.Errorf("hash[%q] differs: %q vs %q", k, v, h2[k])
}
}
}
// TestHashLocalTree_SymlinkSkipped: symlinks should not appear in the
// hash map — same posture as the tar walker. Otherwise a hostile plugin
// could include a symlink whose hash changes when its target changes,
// silently flipping classification.
func TestHashLocalTree_SymlinkSkipped(t *testing.T) {
dir := t.TempDir()
if err := os.WriteFile(filepath.Join(dir, "real.txt"), []byte("ok"), 0o644); err != nil {
t.Fatal(err)
}
target := filepath.Join(t.TempDir(), "target")
if err := os.WriteFile(target, []byte("outside"), 0o644); err != nil {
t.Fatal(err)
}
if err := os.Symlink(target, filepath.Join(dir, "link")); err != nil {
t.Fatal(err)
}
h, err := hashLocalTree(dir)
if err != nil {
t.Fatal(err)
}
if _, exists := h["link"]; exists {
t.Errorf("symlink leaked into hash map: %v", h)
}
if _, exists := h["real.txt"]; !exists {
t.Errorf("real.txt missing from hash map: %v", h)
}
}
// TestShQuote: the classifier injects livePath into a shell command via
// docker exec. Path must be quoted to handle pluginName entries with
// hyphens (which are safe but exercised here) and any future special-
// character edge case. Pin the safe-vs-quoted boundary.
func TestShQuote(t *testing.T) {
cases := []struct {
in, want string
}{
{"foo", "foo"},
{"/configs/plugins/foo-bar", "/configs/plugins/foo-bar"},
{"with space", "'with space'"},
{"with'quote", "'with'\\''quote'"},
{"$envvar", "'$envvar'"},
{"path/with/dots.txt", "path/with/dots.txt"},
}
for _, tc := range cases {
if got := shQuote(tc.in); got != tc.want {
t.Errorf("shQuote(%q) = %q; want %q", tc.in, got, tc.want)
}
}
}
@@ -276,7 +276,22 @@ 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 {
// Hot-reload classifier (molecule-core#112) — decide BEFORE the
// install whether this update can skip restartFunc. SKILL.md
// content changes are filesystem-visible to Claude Code on the
// next Skill invocation; hooks / settings.json / plugin.yaml /
// added-or-removed files need a container restart.
// Classifier reads live tree from container; on any read error
// it returns kindCold so we never hot-reload speculatively.
kind, _ := h.classifyInstallChanges(ctx, containerName, r.StagedDir, r.PluginName)
// 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"})
}
@@ -284,7 +299,11 @@ func (h *PluginsHandler) deliverToContainer(ctx context.Context, workspaceID str
"chown", "-R", "1000:1000", "/configs/plugins/" + r.PluginName,
})
if h.restartFunc != nil {
go h.restartFunc(workspaceID)
if kind == classifyKindSkillContentOnly {
log.Printf("Plugin install: %s → workspace %s — SKILL-content-only update, SKIPPING restart", r.PluginName, workspaceID)
} else {
go h.restartFunc(workspaceID)
}
}
return nil
}
@@ -207,20 +207,25 @@ func TestStartSweeper_TransientErrorDoesNotCrashLoop(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
// 50ms ticker so the second cycle fires quickly enough for the test.
// We re-export SweepInterval as a const, but tests use the public
// StartSweeper that takes its own interval — wait, the public
// StartSweeper signature uses the package-level SweepInterval. Hmm,
// this means the test takes ~5 minutes. Let me reconsider.
//
// (We patch the test below to just look at the immediate-sweep call
// + an error path, since the immediate call is enough to prove the
// "error doesn't crash" contract — the loop continues afterward
// regardless of timing.)
// Capture metric baseline so we can wait for the error counter to
// settle before returning — otherwise this test's leaked metric
// write races with the next test's metricDelta() baseline read and
// causes a non-deterministic +1 leak (manifests as
// TestStartSweeper_RecordsMetricsOnSuccess: "error counter delta=1,
// want 0"). cycleDone fires inside the fake's Sweep defer, BEFORE
// sweepOnce records the error metric — so cancel() right after
// waitForCycle is too early.
_, _, deltaError := metricDelta(t)
go pendinguploads.StartSweeper(ctx, store, time.Hour)
// Wait for the first (errored) cycle.
store.waitForCycle(t, 1, 2*time.Second)
// Wait for the goroutine to record the error metric. After this
// returns, sweepOnce has fully completed and a subsequent cancel()
// stops the loop on the next select pass with no in-flight metric
// writes outstanding.
waitForMetricDelta(t, deltaError, 1, 2*time.Second)
// Cancel — the goroutine returns cleanly, proving the error path
// didn't crash the loop. Without this fix the goroutine would have
// either panicked (process abort visible at exit) or stuck (this