From a1afbce84cee5ed5f859287359e3c657cbc00604 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sun, 17 May 2026 20:46:29 -0700 Subject: [PATCH] feat(mcp): embed Python engine and extract to user cache U2 of the Claude Desktop .mcpb bundle plan. - internal/engine/embed.go embeds the vendored Python tree at build time via //go:embed all:vendored. The engine package owns the embed because Go's directive cannot reach outside its own package directory; sync and gitignore paths are updated to match (internal/engine/vendored/ in place of mcp/vendored/). - internal/engine/extract.go materializes the embed into /last30days-pp-mcp// with a .version sentinel that short-circuits re-extraction. Atomic rename from a .tmp sibling means a partial extraction can never be mistaken for complete. Concurrent first-call extractions serialize behind a per-cache-dir sync.Once. - EnsureUserCache honors a LAST30DAYS_CACHE_DIR env override for locked-down filesystems; the override is named in extract errors. - internal/engine/extract_test.go covers happy path, sentinel skip, version bump, 10-goroutine race, empty-version rejection, unwritable cache parent, and the env override (7 tests, all passing). - A tracked vendored/.gitkeep anchors the embed path so the directive matches even before scripts/sync-engine.sh runs. --- .gitignore | 6 +- mcp/.gitignore | 5 +- mcp/README.md | 2 +- mcp/internal/engine/embed.go | 26 ++++ mcp/internal/engine/extract.go | 168 ++++++++++++++++++++++++++ mcp/internal/engine/extract_test.go | 167 +++++++++++++++++++++++++ mcp/internal/engine/vendored/.gitkeep | 2 + mcp/scripts/sync-engine.sh | 7 +- 8 files changed, 378 insertions(+), 5 deletions(-) create mode 100644 mcp/internal/engine/embed.go create mode 100644 mcp/internal/engine/extract.go create mode 100644 mcp/internal/engine/extract_test.go create mode 100644 mcp/internal/engine/vendored/.gitkeep diff --git a/.gitignore b/.gitignore index aa03da0..877b2e7 100644 --- a/.gitignore +++ b/.gitignore @@ -28,7 +28,11 @@ htmlcov/ # Go MCP bundle build outputs - source of truth for vendored/ stays under # skills/last30days/scripts/; build/ holds cross-compiled binaries + .mcpb files. -/mcp/vendored/ +# vendored/ lives inside the engine package because //go:embed cannot reach +# outside its own package directory; the .gitkeep anchor stays tracked so +# the embed pattern always finds a match even before sync-engine runs. +/mcp/internal/engine/vendored/* +!/mcp/internal/engine/vendored/.gitkeep /mcp/build/ # Internal planning docs (ce:plan output) — keep local, don't publish diff --git a/mcp/.gitignore b/mcp/.gitignore index 3852985..22541f7 100644 --- a/mcp/.gitignore +++ b/mcp/.gitignore @@ -1,6 +1,9 @@ # Mirror of skills/last30days/scripts/, populated by scripts/sync-engine.sh. # Source of truth lives in the Python skill; never commit the mirror. -vendored/ +# Lives inside internal/engine/ because //go:embed cannot reach outside +# its own package directory. +internal/engine/vendored/* +!internal/engine/vendored/.gitkeep # Local build output: cross-compiled binaries and packaged .mcpb files. build/ diff --git a/mcp/README.md b/mcp/README.md index 72a5f48..2f1d260 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -9,7 +9,7 @@ The MCP server exposes a single `research` tool that mirrors the `/last30days and returns +// the cache path. If the sentinel file already records the same version the +// directory is reused without rewriting. version must be non-empty so the +// cache layout always namespaces by version. +// +// Extraction writes to a sibling .tmp directory and renames it on success +// so a partial extraction can never be mistaken for a complete one. Concurrent +// callers within the same process serialize behind a per-cache-dir sync.Once +// so the rename happens exactly once. +func Ensure(src fs.FS, baseDir, version string) (string, error) { + if version == "" { + return "", errors.New("engine: version is required") + } + cacheDir := filepath.Join(baseDir, cacheSubdir, version) + + once := getOnce(cacheDir) + var extractErr error + once.Do(func() { + extractErr = ensureLocked(src, cacheDir, version) + }) + if extractErr != nil { + // Reset the sync.Once so a follow-up call can retry rather than + // permanently caching the error. Retry is the right default when + // the failure is transient (e.g., disk full, parent dir restored). + resetOnce(cacheDir) + return "", extractErr + } + return cacheDir, nil +} + +// EnsureUserCache wraps Ensure with the OS user cache dir (or the +// LAST30DAYS_CACHE_DIR override) as base. Production callers use this; tests +// use Ensure with an explicit temp dir. +func EnsureUserCache(src fs.FS, version string) (string, error) { + if override := os.Getenv(CacheEnvOverride); override != "" { + return Ensure(src, override, version) + } + base, err := os.UserCacheDir() + if err != nil { + return "", fmt.Errorf("engine: resolve user cache dir (set %s to override): %w", CacheEnvOverride, err) + } + return Ensure(src, base, version) +} + +func ensureLocked(src fs.FS, cacheDir, version string) error { + if sentinelMatches(cacheDir, version) { + return nil + } + tmpDir := cacheDir + ".tmp" + if err := os.RemoveAll(tmpDir); err != nil { + return fmt.Errorf("engine: clean tmp cache: %w", err) + } + if err := os.MkdirAll(tmpDir, 0o755); err != nil { + return fmt.Errorf("engine: create tmp cache (%s, set %s to override): %w", tmpDir, CacheEnvOverride, err) + } + if err := extractAll(src, tmpDir); err != nil { + _ = os.RemoveAll(tmpDir) + return err + } + sentinel := filepath.Join(tmpDir, SentinelFilename) + if err := os.WriteFile(sentinel, []byte(version), 0o644); err != nil { + _ = os.RemoveAll(tmpDir) + return fmt.Errorf("engine: write sentinel: %w", err) + } + if err := os.RemoveAll(cacheDir); err != nil { + _ = os.RemoveAll(tmpDir) + return fmt.Errorf("engine: clean old cache: %w", err) + } + if err := os.Rename(tmpDir, cacheDir); err != nil { + _ = os.RemoveAll(tmpDir) + return fmt.Errorf("engine: promote tmp cache: %w", err) + } + return nil +} + +func sentinelMatches(cacheDir, version string) bool { + data, err := os.ReadFile(filepath.Join(cacheDir, SentinelFilename)) + if err != nil { + return false + } + return string(data) == version +} + +func extractAll(src fs.FS, dst string) error { + return fs.WalkDir(src, ".", func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if path == "." { + return nil + } + target := filepath.Join(dst, path) + if d.IsDir() { + return os.MkdirAll(target, 0o755) + } + return copyEmbeddedFile(src, path, target) + }) +} + +func copyEmbeddedFile(src fs.FS, srcPath, dst string) error { + in, err := src.Open(srcPath) + if err != nil { + return fmt.Errorf("engine: open %s: %w", srcPath, err) + } + defer func() { _ = in.Close() }() + if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + return fmt.Errorf("engine: ensure parent of %s: %w", dst, err) + } + out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) + if err != nil { + return fmt.Errorf("engine: create %s: %w", dst, err) + } + defer func() { _ = out.Close() }() + if _, err := io.Copy(out, in); err != nil { + return fmt.Errorf("engine: write %s: %w", dst, err) + } + return nil +} + +// onceRegistry serializes first-call extraction per cache directory so the +// rename in ensureLocked happens exactly once across goroutines. +var ( + onceMu sync.Mutex + onceRegistry = map[string]*sync.Once{} +) + +func getOnce(cacheDir string) *sync.Once { + onceMu.Lock() + defer onceMu.Unlock() + if o, ok := onceRegistry[cacheDir]; ok { + return o + } + o := &sync.Once{} + onceRegistry[cacheDir] = o + return o +} + +func resetOnce(cacheDir string) { + onceMu.Lock() + defer onceMu.Unlock() + delete(onceRegistry, cacheDir) +} diff --git a/mcp/internal/engine/extract_test.go b/mcp/internal/engine/extract_test.go new file mode 100644 index 0000000..c4c0a3a --- /dev/null +++ b/mcp/internal/engine/extract_test.go @@ -0,0 +1,167 @@ +package engine + +import ( + "os" + "path/filepath" + "sync" + "testing" + "testing/fstest" +) + +func newTestFS() fstest.MapFS { + return fstest.MapFS{ + "last30days.py": &fstest.MapFile{Data: []byte("# last30days entry\n"), Mode: 0o644}, + "lib/__init__.py": &fstest.MapFile{Data: []byte(""), Mode: 0o644}, + "lib/env.py": &fstest.MapFile{Data: []byte("# env helpers\n"), Mode: 0o644}, + } +} + +func TestEnsureExtractsEngine(t *testing.T) { + src := newTestFS() + base := t.TempDir() + + cacheDir, err := Ensure(src, base, "v1") + if err != nil { + t.Fatalf("Ensure: %v", err) + } + if cacheDir != filepath.Join(base, cacheSubdir, "v1") { + t.Fatalf("cacheDir = %q, want %q", cacheDir, filepath.Join(base, cacheSubdir, "v1")) + } + mustReadFile(t, filepath.Join(cacheDir, "last30days.py"), "# last30days entry\n") + mustReadFile(t, filepath.Join(cacheDir, "lib/env.py"), "# env helpers\n") + mustReadFile(t, filepath.Join(cacheDir, SentinelFilename), "v1") +} + +func TestEnsureSkipsWhenSentinelMatches(t *testing.T) { + src := newTestFS() + base := t.TempDir() + + cacheDir, err := Ensure(src, base, "v1") + if err != nil { + t.Fatalf("first Ensure: %v", err) + } + target := filepath.Join(cacheDir, "last30days.py") + info1, err := os.Stat(target) + if err != nil { + t.Fatalf("stat: %v", err) + } + + // Reset the sync.Once so a second call would re-extract if not for the + // sentinel short-circuit. Without the reset, sync.Once would skip the + // extraction regardless of sentinel state. + resetOnce(cacheDir) + + if _, err := Ensure(src, base, "v1"); err != nil { + t.Fatalf("second Ensure: %v", err) + } + info2, err := os.Stat(target) + if err != nil { + t.Fatalf("stat second: %v", err) + } + if !info2.ModTime().Equal(info1.ModTime()) { + t.Fatalf("expected file untouched on sentinel match; got mtime %v -> %v", info1.ModTime(), info2.ModTime()) + } +} + +func TestEnsureReExtractsOnVersionChange(t *testing.T) { + v1 := fstest.MapFS{ + "last30days.py": &fstest.MapFile{Data: []byte("v1\n"), Mode: 0o644}, + } + v2 := fstest.MapFS{ + "last30days.py": &fstest.MapFile{Data: []byte("v2\n"), Mode: 0o644}, + } + base := t.TempDir() + + cache1, err := Ensure(v1, base, "v1") + if err != nil { + t.Fatalf("Ensure v1: %v", err) + } + cache2, err := Ensure(v2, base, "v2") + if err != nil { + t.Fatalf("Ensure v2: %v", err) + } + if cache1 == cache2 { + t.Fatalf("expected distinct cache dirs per version, got %q == %q", cache1, cache2) + } + mustReadFile(t, filepath.Join(cache1, "last30days.py"), "v1\n") + mustReadFile(t, filepath.Join(cache2, "last30days.py"), "v2\n") +} + +func TestEnsureConcurrentFirstCall(t *testing.T) { + src := newTestFS() + base := t.TempDir() + + const goroutines = 10 + var wg sync.WaitGroup + wg.Add(goroutines) + results := make([]string, goroutines) + errs := make([]error, goroutines) + for i := 0; i < goroutines; i++ { + i := i + go func() { + defer wg.Done() + results[i], errs[i] = Ensure(src, base, "v1") + }() + } + wg.Wait() + + for i, err := range errs { + if err != nil { + t.Fatalf("goroutine %d: %v", i, err) + } + } + for i := 1; i < goroutines; i++ { + if results[i] != results[0] { + t.Fatalf("goroutine 0 saw %q, goroutine %d saw %q", results[0], i, results[i]) + } + } + mustReadFile(t, filepath.Join(results[0], "last30days.py"), "# last30days entry\n") +} + +func TestEnsureRejectsEmptyVersion(t *testing.T) { + if _, err := Ensure(newTestFS(), t.TempDir(), ""); err == nil { + t.Fatal("expected error for empty version") + } +} + +func TestEnsureReturnsErrorWhenCacheUnwritable(t *testing.T) { + // Place the cache root at a path that cannot exist (a regular file). + // MkdirAll will refuse and Ensure must surface a wrapped error. + base := t.TempDir() + blocker := filepath.Join(base, "blocker") + if err := os.WriteFile(blocker, []byte("not a dir"), 0o644); err != nil { + t.Fatalf("setup: %v", err) + } + + _, err := Ensure(newTestFS(), blocker, "v1") + if err == nil { + t.Fatal("expected error when cache parent is not a directory") + } +} + +func TestEnsureUserCacheHonorsOverride(t *testing.T) { + override := t.TempDir() + t.Setenv(CacheEnvOverride, override) + + src := newTestFS() + cacheDir, err := EnsureUserCache(src, "v1") + if err != nil { + t.Fatalf("EnsureUserCache: %v", err) + } + want := filepath.Join(override, cacheSubdir, "v1") + if cacheDir != want { + t.Fatalf("cacheDir = %q, want %q", cacheDir, want) + } + mustReadFile(t, filepath.Join(cacheDir, "last30days.py"), "# last30days entry\n") +} + +func mustReadFile(t *testing.T, path, want string) { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + if string(data) != want { + t.Fatalf("%s: got %q, want %q", path, string(data), want) + } +} diff --git a/mcp/internal/engine/vendored/.gitkeep b/mcp/internal/engine/vendored/.gitkeep new file mode 100644 index 0000000..5e52b31 --- /dev/null +++ b/mcp/internal/engine/vendored/.gitkeep @@ -0,0 +1,2 @@ +Populated at build time by scripts/sync-engine.sh. +Source of truth: skills/last30days/scripts/. diff --git a/mcp/scripts/sync-engine.sh b/mcp/scripts/sync-engine.sh index d0b51d6..0f04012 100755 --- a/mcp/scripts/sync-engine.sh +++ b/mcp/scripts/sync-engine.sh @@ -11,15 +11,18 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" MCP_DIR="$(cd "${SCRIPT_DIR}/.." && pwd)" REPO_ROOT="$(cd "${MCP_DIR}/.." && pwd)" ENGINE_SRC="${REPO_ROOT}/skills/last30days/scripts" -VENDORED="${MCP_DIR}/vendored" +# Embed path must live inside the consuming package (Go //go:embed cannot +# reach outside its own directory tree), so vendored/ sits under engine/. +VENDORED="${MCP_DIR}/internal/engine/vendored" if [ ! -f "${ENGINE_SRC}/last30days.py" ]; then echo "sync-engine: ${ENGINE_SRC}/last30days.py not found" >&2 exit 1 fi -rm -rf "${VENDORED}" mkdir -p "${VENDORED}" +# Clear stale content while keeping the .gitkeep that anchors the embed path. +find "${VENDORED}" -mindepth 1 -not -name ".gitkeep" -delete # Copy the entry script and the lib/ tree (modules + lib/vendor/). cp "${ENGINE_SRC}/last30days.py" "${VENDORED}/last30days.py"