From 459df965c4bc0939c19feaa5f2eecf14e8fcbf59 Mon Sep 17 00:00:00 2001 From: shakestzd Date: Sun, 19 Apr 2026 19:43:21 -0400 Subject: [PATCH] fix(cli-codex): auto-install plugin to ~/.codex/plugins/cache after marketplace add MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `codex marketplace add` only registers the marketplace source in config.toml. The plugin isn't installed to Codex's runtime cache (~/.codex/plugins/cache////) until the user manually opens the TUI, runs /plugin, finds the plugin in the list, and clicks install. Empirically verified. This is a UX wart for `htmlgraph codex --init`/`--dev`: users expect "init → launch → skills work", not "init → launch → /plugin → find → install → now skills work". Fix: parse /.agents/plugins/marketplace.json to resolve marketplace name + plugin name + source subpath, then read the plugin version from .codex-plugin/plugin.json, and copy the plugin source tree directly to the cache layout Codex expects: ~/.codex/plugins/cache//// The copy is idempotent (clears stale cache first, then copies fresh). Wired into both runCodexInit and launchCodexDev. After this fix, a fresh run of `htmlgraph codex --init` is sufficient to make our skills discoverable via $autocomplete on the next codex launch — no manual /plugin step required. Tests added: - TestParseCodexMarketplaceJSON — parse marketplace.json structure - TestParseCodexPluginVersion — extract version from plugin.json - TestCopyDirCreatesExpectedLayout — copyDir creates correct structure - TestCopyDirIdempotent — copyDir is idempotent - TestInstallCodexPluginToCacheCreatesExpectedLayout — full cache install - TestInstallCodexPluginToCacheIdempotent — install is idempotent Fixes bug-d25198ff. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/htmlgraph/codex.go | 195 ++++++++++++++++++++++++ cmd/htmlgraph/codex_test.go | 291 ++++++++++++++++++++++++++++++++++++ 2 files changed, 486 insertions(+) diff --git a/cmd/htmlgraph/codex.go b/cmd/htmlgraph/codex.go index 9217eaf4..40697bac 100644 --- a/cmd/htmlgraph/codex.go +++ b/cmd/htmlgraph/codex.go @@ -2,7 +2,9 @@ package main import ( "bufio" + "encoding/json" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -227,6 +229,157 @@ func promptYesNo(question string, yes bool) bool { return answer == "y" || answer == "yes" } +// copyDir recursively copies src directory to dst. +// If dst exists, it is removed first (idempotent). +// Uses filepath.Walk + os.MkdirAll + io.Copy for portability. +func copyDir(src, dst string) error { + // Remove destination if it exists + if err := os.RemoveAll(dst); err != nil { + return fmt.Errorf("remove existing destination: %w", err) + } + + // Walk the source directory + return filepath.Walk(src, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Compute relative path + rel, err := filepath.Rel(src, path) + if err != nil { + return err + } + + // Destination path + dstPath := filepath.Join(dst, rel) + + if info.IsDir() { + // Create directory + return os.MkdirAll(dstPath, info.Mode()) + } + + // Copy file + srcFile, err := os.Open(path) + if err != nil { + return err + } + defer srcFile.Close() + + // Create parent directories if needed + if err := os.MkdirAll(filepath.Dir(dstPath), 0755); err != nil { + return err + } + + dstFile, err := os.Create(dstPath) + if err != nil { + return err + } + defer dstFile.Close() + + if _, err := io.Copy(dstFile, srcFile); err != nil { + return err + } + + return os.Chmod(dstPath, info.Mode()) + }) +} + +// parseCodexMarketplaceJSON parses the marketplace.json file at +// /.agents/plugins/marketplace.json and returns: +// - marketplace name (e.g., "htmlgraph") +// - plugin name (e.g., "htmlgraph") +// - plugin source subpath (e.g., "htmlgraph", corresponding to "./htmlgraph" in the JSON) +func parseCodexMarketplaceJSON(marketplaceRoot string) (mktName, pluginName, pluginSourceSubpath string, err error) { + jsonPath := filepath.Join(marketplaceRoot, ".agents", "plugins", "marketplace.json") + data, err := os.ReadFile(jsonPath) + if err != nil { + return "", "", "", fmt.Errorf("read marketplace.json at %s: %w", jsonPath, err) + } + + var mkt struct { + Name string `json:"name"` + Plugins []struct { + Name string `json:"name"` + Source struct { + Path string `json:"path"` + } `json:"source"` + } `json:"plugins"` + } + + if err := json.Unmarshal(data, &mkt); err != nil { + return "", "", "", fmt.Errorf("parse marketplace.json: %w", err) + } + + if len(mkt.Plugins) == 0 { + return "", "", "", fmt.Errorf("marketplace.json has no plugins defined") + } + + mktName = mkt.Name + pluginName = mkt.Plugins[0].Name + // Remove leading "./" if present + pluginSourceSubpath = strings.TrimPrefix(mkt.Plugins[0].Source.Path, "./") + + return mktName, pluginName, pluginSourceSubpath, nil +} + +// parseCodexPluginVersion reads the plugin version from the plugin's +// .codex-plugin/plugin.json manifest at the given marketplace root + subpath. +func parseCodexPluginVersion(marketplaceRoot, pluginSourceSubpath string) (string, error) { + jsonPath := filepath.Join(marketplaceRoot, pluginSourceSubpath, ".codex-plugin", "plugin.json") + data, err := os.ReadFile(jsonPath) + if err != nil { + return "", fmt.Errorf("read plugin.json at %s: %w", jsonPath, err) + } + + var manifest struct { + Version string `json:"version"` + } + + if err := json.Unmarshal(data, &manifest); err != nil { + return "", fmt.Errorf("parse plugin.json: %w", err) + } + + if manifest.Version == "" { + return "", fmt.Errorf("plugin.json has no version field") + } + + return manifest.Version, nil +} + +// installCodexPluginToCache copies the plugin source tree to Codex's expected +// cache layout so the plugin is immediately usable without the user manually +// invoking /plugin → install in the TUI. +// +// Cache path layout per Codex docs: +// +// ~/.codex/plugins/cache//// +// +// For example: +// +// ~/.codex/plugins/cache/htmlgraph/htmlgraph/0.55.5/ +func installCodexPluginToCache(marketplaceRoot, marketplaceName, pluginName, pluginSourceSubpath string) error { + // Get the plugin version + version, err := parseCodexPluginVersion(marketplaceRoot, pluginSourceSubpath) + if err != nil { + return fmt.Errorf("determine plugin version: %w", err) + } + + home, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("get home directory: %w", err) + } + + src := filepath.Join(marketplaceRoot, pluginSourceSubpath) + dst := filepath.Join(home, ".codex", "plugins", "cache", marketplaceName, pluginName, version) + + // Idempotent: remove dst if it exists, then copy fresh + if err := copyDir(src, dst); err != nil { + return fmt.Errorf("copy plugin to cache at %s: %w", dst, err) + } + + return nil +} + // codexCmd returns the cobra command for `htmlgraph codex`. func codexCmd() *cobra.Command { var init_, continue_, dev, cleanup, dryRun, yes bool @@ -300,6 +453,32 @@ func runCodexInit(yes, dryRun bool) error { fmt.Println("HtmlGraph Codex marketplace is already installed.") } + // Phase 1b: Install plugin to cache. + // This only works if we have a project root (for --init from the repo). + // If running --init from outside the repo, we assume the sparse checkout has + // already populated the marketplace source, but we won't install to cache + // (that requires knowing the source path structure). + if projectRoot, err := resolveProjectRoot(); err == nil { + localMarketplace := filepath.Join(projectRoot, "packages", "codex-marketplace") + if _, statErr := os.Stat(localMarketplace); statErr == nil { + // We have the marketplace source locally; install to cache + mktName, plgName, plgSub, parseErr := parseCodexMarketplaceJSON(localMarketplace) + if parseErr != nil { + fmt.Fprintf(os.Stderr, "warning: could not parse marketplace.json: %v\n", parseErr) + } else { + if dryRun { + fmt.Printf("[dry-run] would install plugin to ~/.codex/plugins/cache/%s/%s//\n", mktName, plgName) + } else { + if installErr := installCodexPluginToCache(localMarketplace, mktName, plgName, plgSub); installErr != nil { + fmt.Fprintf(os.Stderr, "warning: could not install plugin to cache: %v\n", installErr) + } else { + fmt.Printf("Plugin installed to ~/.codex/plugins/cache/%s/%s//\n", mktName, plgName) + } + } + } + } + } + // Phase 2: Check and optionally enable codex_hooks feature flag. // This runs on every --init so partial setups can be repaired. if !isCodexHooksEnabledAt(configPath) { @@ -406,6 +585,22 @@ func launchCodexDev(resumeID string, cleanup, dryRun bool, extraArgs []string) e fmt.Println("Local marketplace already registered — proceeding.") } + // Install plugin to cache (so it's available without manual /plugin install) + if !dryRun { + mktName, plgName, plgSub, parseErr := parseCodexMarketplaceJSON(localMarketplace) + if parseErr != nil { + fmt.Fprintf(os.Stderr, "warning: could not parse marketplace.json: %v\n", parseErr) + } else { + if installErr := installCodexPluginToCache(localMarketplace, mktName, plgName, plgSub); installErr != nil { + fmt.Fprintf(os.Stderr, "warning: could not install plugin to cache: %v\n", installErr) + } else { + fmt.Printf("Plugin installed to ~/.codex/plugins/cache/%s/%s//\n", mktName, plgName) + } + } + } else { + fmt.Printf("[dry-run] would install plugin to ~/.codex/plugins/cache////\n") + } + projectRoot, _ := resolveProjectRoot() if dryRun { diff --git a/cmd/htmlgraph/codex_test.go b/cmd/htmlgraph/codex_test.go index 8523ada5..a01a95b6 100644 --- a/cmd/htmlgraph/codex_test.go +++ b/cmd/htmlgraph/codex_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "encoding/json" "os" "path/filepath" "strings" @@ -457,3 +458,293 @@ func TestRemoveCodexHtmlgraphRegistrationsNonexistentFile(t *testing.T) { t.Errorf("expected removed=false for non-existent file, got true") } } + +// TestParseCodexMarketplaceJSON verifies that parseCodexMarketplaceJSON correctly +// extracts marketplace name, plugin name, and plugin source subpath from marketplace.json. +func TestParseCodexMarketplaceJSON(t *testing.T) { + tmpdir := t.TempDir() + + // Create the directory structure + pluginsDir := filepath.Join(tmpdir, ".agents", "plugins") + if err := os.MkdirAll(pluginsDir, 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + // Create marketplace.json + marketplaceJSON := map[string]interface{}{ + "name": "htmlgraph", + "interface": map[string]interface{}{ + "displayName": "HtmlGraph", + }, + "plugins": []map[string]interface{}{ + { + "name": "htmlgraph", + "source": map[string]interface{}{ + "source": "local", + "path": "./htmlgraph", + }, + "policy": map[string]interface{}{ + "installation": "AVAILABLE", + "authentication": "ON_INSTALL", + }, + "category": "Development Tools", + }, + }, + } + + data, err := json.Marshal(marketplaceJSON) + if err != nil { + t.Fatalf("json.Marshal: %v", err) + } + + marketplaceFile := filepath.Join(pluginsDir, "marketplace.json") + if err := os.WriteFile(marketplaceFile, data, 0644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + // Parse and verify + mktName, plgName, plgSub, err := parseCodexMarketplaceJSON(tmpdir) + if err != nil { + t.Fatalf("parseCodexMarketplaceJSON: %v", err) + } + + if mktName != "htmlgraph" { + t.Errorf("expected marketplace name 'htmlgraph', got %q", mktName) + } + if plgName != "htmlgraph" { + t.Errorf("expected plugin name 'htmlgraph', got %q", plgName) + } + if plgSub != "htmlgraph" { + t.Errorf("expected plugin subpath 'htmlgraph' (without ./), got %q", plgSub) + } +} + +// TestParseCodexPluginVersion verifies that parseCodexPluginVersion correctly +// extracts the version from plugin.json. +func TestParseCodexPluginVersion(t *testing.T) { + tmpdir := t.TempDir() + + // Create the directory structure + pluginDir := filepath.Join(tmpdir, "htmlgraph", ".codex-plugin") + if err := os.MkdirAll(pluginDir, 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + // Create plugin.json + pluginJSON := map[string]interface{}{ + "version": "0.55.5", + "name": "htmlgraph", + } + + data, err := json.Marshal(pluginJSON) + if err != nil { + t.Fatalf("json.Marshal: %v", err) + } + + pluginFile := filepath.Join(pluginDir, "plugin.json") + if err := os.WriteFile(pluginFile, data, 0644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + // Parse and verify + version, err := parseCodexPluginVersion(tmpdir, "htmlgraph") + if err != nil { + t.Fatalf("parseCodexPluginVersion: %v", err) + } + + if version != "0.55.5" { + t.Errorf("expected version '0.55.5', got %q", version) + } +} + +// TestCopyDirCreatesExpectedLayout verifies that copyDir successfully copies +// a source directory tree to a destination, creating all necessary subdirectories. +func TestCopyDirCreatesExpectedLayout(t *testing.T) { + tmpdir := t.TempDir() + + // Create source directory with some files and subdirectories + srcDir := filepath.Join(tmpdir, "src") + if err := os.MkdirAll(filepath.Join(srcDir, "subdir"), 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + // Create some test files + files := map[string]string{ + "file1.txt": "content1", + "subdir/file2.txt": "content2", + "subdir/file3.json": `{"key": "value"}`, + } + + for relPath, content := range files { + fullPath := filepath.Join(srcDir, relPath) + if err := os.WriteFile(fullPath, []byte(content), 0644); err != nil { + t.Fatalf("WriteFile %s: %v", relPath, err) + } + } + + // Copy to destination + dstDir := filepath.Join(tmpdir, "dst") + if err := copyDir(srcDir, dstDir); err != nil { + t.Fatalf("copyDir: %v", err) + } + + // Verify all files exist and have correct content + for relPath, expectedContent := range files { + fullPath := filepath.Join(dstDir, relPath) + content, err := os.ReadFile(fullPath) + if err != nil { + t.Fatalf("ReadFile %s: %v", relPath, err) + } + if string(content) != expectedContent { + t.Errorf("file %s: expected %q, got %q", relPath, expectedContent, string(content)) + } + } +} + +// TestCopyDirIdempotent verifies that copyDir is idempotent — calling it twice +// with the same destination produces identical results (destination is cleared first). +func TestCopyDirIdempotent(t *testing.T) { + tmpdir := t.TempDir() + + // Create source directory with a file + srcDir := filepath.Join(tmpdir, "src") + if err := os.MkdirAll(srcDir, 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + testFile := filepath.Join(srcDir, "test.txt") + if err := os.WriteFile(testFile, []byte("content"), 0644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + dstDir := filepath.Join(tmpdir, "dst") + + // First copy + if err := copyDir(srcDir, dstDir); err != nil { + t.Fatalf("first copyDir: %v", err) + } + data1, err := os.ReadFile(filepath.Join(dstDir, "test.txt")) + if err != nil { + t.Fatalf("ReadFile after first copy: %v", err) + } + + // Second copy (should be idempotent) + if err := copyDir(srcDir, dstDir); err != nil { + t.Fatalf("second copyDir: %v", err) + } + data2, err := os.ReadFile(filepath.Join(dstDir, "test.txt")) + if err != nil { + t.Fatalf("ReadFile after second copy: %v", err) + } + + if !bytes.Equal(data1, data2) { + t.Errorf("idempotency check failed: content differs after second copy") + } +} + +// TestInstallCodexPluginToCacheCreatesExpectedLayout verifies that +// installCodexPluginToCache creates the expected cache layout. +func TestInstallCodexPluginToCacheCreatesExpectedLayout(t *testing.T) { + tmpdir := t.TempDir() + + // Set HOME to tmpdir for this test so UserHomeDir() returns tmpdir + oldHome := os.Getenv("HOME") + defer os.Setenv("HOME", oldHome) + os.Setenv("HOME", tmpdir) + + // Create a fake marketplace with plugin.json and a test file + marketplaceRoot := filepath.Join(tmpdir, "marketplace") + pluginSourceDir := filepath.Join(marketplaceRoot, "htmlgraph") + if err := os.MkdirAll(filepath.Join(pluginSourceDir, ".codex-plugin"), 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + // Create plugin.json with version + pluginJSON := map[string]interface{}{ + "version": "0.55.5", + } + data, _ := json.Marshal(pluginJSON) + if err := os.WriteFile(filepath.Join(pluginSourceDir, ".codex-plugin", "plugin.json"), data, 0644); err != nil { + t.Fatalf("WriteFile plugin.json: %v", err) + } + + // Create a test file in the plugin + testFile := filepath.Join(pluginSourceDir, "test.md") + if err := os.WriteFile(testFile, []byte("test content"), 0644); err != nil { + t.Fatalf("WriteFile test.md: %v", err) + } + + // Install to cache + if err := installCodexPluginToCache(marketplaceRoot, "htmlgraph", "htmlgraph", "htmlgraph"); err != nil { + t.Fatalf("installCodexPluginToCache: %v", err) + } + + // Verify cache layout + expectedCachePath := filepath.Join(tmpdir, ".codex", "plugins", "cache", "htmlgraph", "htmlgraph", "0.55.5") + expectedPluginJSON := filepath.Join(expectedCachePath, ".codex-plugin", "plugin.json") + if _, err := os.Stat(expectedPluginJSON); err != nil { + t.Fatalf("cache layout check failed: %s does not exist: %v", expectedPluginJSON, err) + } + + expectedTestFile := filepath.Join(expectedCachePath, "test.md") + if _, err := os.Stat(expectedTestFile); err != nil { + t.Fatalf("cache layout check failed: %s does not exist: %v", expectedTestFile, err) + } +} + +// TestInstallCodexPluginToCacheIdempotent verifies that installCodexPluginToCache +// is idempotent — calling it twice produces identical cache state. +func TestInstallCodexPluginToCacheIdempotent(t *testing.T) { + tmpdir := t.TempDir() + + // Set HOME to tmpdir + oldHome := os.Getenv("HOME") + defer os.Setenv("HOME", oldHome) + os.Setenv("HOME", tmpdir) + + // Create fake marketplace + marketplaceRoot := filepath.Join(tmpdir, "marketplace") + pluginSourceDir := filepath.Join(marketplaceRoot, "htmlgraph") + if err := os.MkdirAll(filepath.Join(pluginSourceDir, ".codex-plugin"), 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + // Create plugin.json + pluginJSON := map[string]interface{}{ + "version": "0.55.5", + } + data, _ := json.Marshal(pluginJSON) + if err := os.WriteFile(filepath.Join(pluginSourceDir, ".codex-plugin", "plugin.json"), data, 0644); err != nil { + t.Fatalf("WriteFile plugin.json: %v", err) + } + + // Create a test file + testFile := filepath.Join(pluginSourceDir, "test.md") + if err := os.WriteFile(testFile, []byte("test content"), 0644); err != nil { + t.Fatalf("WriteFile test.md: %v", err) + } + + // First install + if err := installCodexPluginToCache(marketplaceRoot, "htmlgraph", "htmlgraph", "htmlgraph"); err != nil { + t.Fatalf("first installCodexPluginToCache: %v", err) + } + + cachePath := filepath.Join(tmpdir, ".codex", "plugins", "cache", "htmlgraph", "htmlgraph", "0.55.5", "test.md") + data1, err := os.ReadFile(cachePath) + if err != nil { + t.Fatalf("ReadFile after first install: %v", err) + } + + // Second install (should be idempotent) + if err := installCodexPluginToCache(marketplaceRoot, "htmlgraph", "htmlgraph", "htmlgraph"); err != nil { + t.Fatalf("second installCodexPluginToCache: %v", err) + } + + data2, err := os.ReadFile(cachePath) + if err != nil { + t.Fatalf("ReadFile after second install: %v", err) + } + + if !bytes.Equal(data1, data2) { + t.Errorf("idempotency check failed: cache content differs after second install") + } +}