← back to Cli Printing Press
fix(cli): always derive install module from filesystem in migrate-skill-metadata (#611)
1b0dc70ac4d7e6f5992683c7fe8e5cfa5aa848a7 · 2026-05-05 01:28:31 -0700 · Trevin Chow
The migration tool was preserving the legacy `command:` field's path verbatim,
which carried stale install URLs for any CLI that had been moved between
categories without its SKILL.md being regenerated. Audit on the public library
found 9 of 44 entries with such drift (e.g., kalshi pointing at
`library/other/kalshi` while actually living under `library/payments/kalshi`).
Switch the migration to always derive the module path from the file's actual
filesystem location and sibling `.printing-press.json` `cli_name`, matching
the behavior of the synthesis path. Drop the legacy `command:` field from
extraction entirely; it has no role anymore. Drop `--strict` mode (it gated
shape variations of input that no longer matter — module path comes from
provenance, env/primaryEnv come from a small, opt-in subset of openclaw JSON,
and structurally-unusual JSON simply produces a no-auth migration).
Other changes:
- Category now derived from the file's path under `library/<category>/<dir>/`
rather than from `.printing-press.json`'s `category` field, which is absent
in 36 of 44 published provenance files. The filesystem path is the canonical
truth for category.
- Resolve symlinks on the supplied root in addition to per-file paths so
mktemp-created roots under `/var/folders/...` (which symlinks to
`/private/var/folders/...` on macOS) don't fail the path-traversal guard.
- `parseLegacyOpenclawJSON` simplified to extract just `requires.env` and
`primaryEnv`. Errors only on malformed JSON.
- New `TestMigrateFile_RerouteCorrectsStalePath` proves the tool replaces a
stale legacy path with the filesystem-derived canonical one.
Verified end-to-end against the public library: 86 migrated + 2 synthesized
+ 0 errored on real CLIs, all 9 previously-stale module paths now correct.
(One additional `errored` is `cli-skills/pp-pagliacci-pizza`, an orphan
with no corresponding library entry; the error surfaces clearly so the
maintainer can clean it up.)
Files touched
M tools/migrate-skill-metadata/main.goM tools/migrate-skill-metadata/main_test.go
Diff
commit 1b0dc70ac4d7e6f5992683c7fe8e5cfa5aa848a7
Author: Trevin Chow <trevin@trevinchow.com>
Date: Tue May 5 01:28:31 2026 -0700
fix(cli): always derive install module from filesystem in migrate-skill-metadata (#611)
The migration tool was preserving the legacy `command:` field's path verbatim,
which carried stale install URLs for any CLI that had been moved between
categories without its SKILL.md being regenerated. Audit on the public library
found 9 of 44 entries with such drift (e.g., kalshi pointing at
`library/other/kalshi` while actually living under `library/payments/kalshi`).
Switch the migration to always derive the module path from the file's actual
filesystem location and sibling `.printing-press.json` `cli_name`, matching
the behavior of the synthesis path. Drop the legacy `command:` field from
extraction entirely; it has no role anymore. Drop `--strict` mode (it gated
shape variations of input that no longer matter — module path comes from
provenance, env/primaryEnv come from a small, opt-in subset of openclaw JSON,
and structurally-unusual JSON simply produces a no-auth migration).
Other changes:
- Category now derived from the file's path under `library/<category>/<dir>/`
rather than from `.printing-press.json`'s `category` field, which is absent
in 36 of 44 published provenance files. The filesystem path is the canonical
truth for category.
- Resolve symlinks on the supplied root in addition to per-file paths so
mktemp-created roots under `/var/folders/...` (which symlinks to
`/private/var/folders/...` on macOS) don't fail the path-traversal guard.
- `parseLegacyOpenclawJSON` simplified to extract just `requires.env` and
`primaryEnv`. Errors only on malformed JSON.
- New `TestMigrateFile_RerouteCorrectsStalePath` proves the tool replaces a
stale legacy path with the filesystem-derived canonical one.
Verified end-to-end against the public library: 86 migrated + 2 synthesized
+ 0 errored on real CLIs, all 9 previously-stale module paths now correct.
(One additional `errored` is `cli-skills/pp-pagliacci-pizza`, an orphan
with no corresponding library entry; the error surfaces clearly so the
maintainer can clean it up.)
---
tools/migrate-skill-metadata/main.go | 279 +++++++++++++++---------------
tools/migrate-skill-metadata/main_test.go | 191 +++++++++-----------
2 files changed, 217 insertions(+), 253 deletions(-)
diff --git a/tools/migrate-skill-metadata/main.go b/tools/migrate-skill-metadata/main.go
index 6b60d72b..f88d5786 100644
--- a/tools/migrate-skill-metadata/main.go
+++ b/tools/migrate-skill-metadata/main.go
@@ -5,14 +5,17 @@
// cli-skills/pp-*/SKILL.md files. For each file:
//
// - If the frontmatter contains a single-line `metadata: '{"openclaw":...}'`,
-// parse the embedded JSON, apply schema corrections (kind: shell -> go,
-// drop command/id/label, derive module from command), and replace that
-// line with a multi-line nested YAML block at canonical indentation.
+// parse the embedded JSON for env/primaryEnv, derive the canonical Go
+// module path from the file's actual filesystem location and sibling
+// `.printing-press.json` provenance, and replace that line with a
+// multi-line nested YAML block. The legacy `command:` field's path is
+// intentionally NOT preserved — it can be stale (CLIs moved between
+// categories without their SKILL.md being updated). The filesystem
+// location plus provenance is the canonical truth.
//
// - If the frontmatter has no `metadata:` line at all (the instacart case),
-// synthesize a block from the sibling `.printing-press.json` provenance
-// (library files) or from the corresponding library entry (cli-skills
-// mirrors) and insert it before the closing `---`.
+// synthesize a block from the same provenance lookup and insert it
+// before the closing `---`.
//
// - If the frontmatter is already in the nested-YAML form, skip without
// writing (idempotent).
@@ -37,7 +40,6 @@ import (
func main() {
dryRun := flag.Bool("dry-run", false, "Print what would change without writing")
- strict := flag.Bool("strict", true, "Refuse files whose JSON doesn't match the expected kind: shell + go-install shape")
verbose := flag.Bool("verbose", false, "Print per-file action lines")
flag.Usage = func() {
fmt.Fprintf(os.Stderr, "usage: migrate-skill-metadata [flags] <library-root>\n\n")
@@ -53,18 +55,29 @@ func main() {
}
root := flag.Arg(0)
- absRoot, err := filepath.Abs(root)
+ // Resolve symlinks on the root too. Each per-file path is run through
+ // filepath.EvalSymlinks before the prefix check; if the root itself
+ // is a symlink (common on macOS where /var -> /private/var, so any
+ // mktemp-created tree under /var/folders/... resolves to /private/...
+ // after EvalSymlinks), an unresolved root would fail the prefix check
+ // for every file.
+ absRoot, err := filepath.EvalSymlinks(root)
if err != nil {
fmt.Fprintf(os.Stderr, "error: cannot resolve root path: %v\n", err)
os.Exit(1)
}
+ absRoot, err = filepath.Abs(absRoot)
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "error: cannot absolutize root path: %v\n", err)
+ os.Exit(1)
+ }
info, err := os.Stat(absRoot)
if err != nil || !info.IsDir() {
fmt.Fprintf(os.Stderr, "error: %s is not a directory\n", absRoot)
os.Exit(1)
}
- report, err := run(absRoot, *dryRun, *strict, *verbose)
+ report, err := run(absRoot, *dryRun, *verbose)
if err != nil {
fmt.Fprintf(os.Stderr, "error: %v\n", err)
os.Exit(1)
@@ -99,7 +112,7 @@ func (r *report) print(w *os.File, dryRun bool) {
}
}
-func run(absRoot string, dryRun, strict, verbose bool) (*report, error) {
+func run(absRoot string, dryRun, verbose bool) (*report, error) {
files, err := discoverSkillFiles(absRoot)
if err != nil {
return nil, err
@@ -132,7 +145,7 @@ func run(absRoot string, dryRun, strict, verbose bool) (*report, error) {
continue
}
- action, err := migrateFile(abs, absRoot, strict, dryRun)
+ action, err := migrateFile(abs, absRoot, dryRun)
switch {
case err != nil:
r.errored++
@@ -183,8 +196,13 @@ func discoverSkillFiles(absRoot string) ([]string, error) {
}
// migrateFile applies the migration to one SKILL.md, returning the action
-// taken: "migrated", "synthesized", "skipped", or "" plus an error.
-func migrateFile(absPath, absRoot string, strict, dryRun bool) (string, error) {
+// taken: "migrated", "synthesized", "skipped", or "" plus an error. The
+// install module path always comes from the file's actual filesystem
+// location and sibling `.printing-press.json` provenance — never from the
+// legacy JSON-string `command:` field, which can be stale (CLIs moved
+// between categories without their SKILL.md being updated). This makes
+// the tool produce correct output by default with no flag to remember.
+func migrateFile(absPath, absRoot string, dryRun bool) (string, error) {
content, err := os.ReadFile(absPath)
if err != nil {
return "", fmt.Errorf("read: %w", err)
@@ -198,19 +216,22 @@ func migrateFile(absPath, absRoot string, strict, dryRun bool) (string, error) {
jsonLineRe := regexp.MustCompile(`(?m)^metadata: '(.+)'\s*$`)
if m := jsonLineRe.FindStringSubmatchIndex(front); m != nil {
- // Idempotency check: if the line already starts with a multi-line
- // nested form, FindStringSubmatchIndex would not have matched. So
- // matching here means we have the legacy JSON-string form.
+ // Legacy JSON-string form. Parse for env/primaryEnv (which the legacy
+ // JSON is the source of truth for) but always derive the module from
+ // provenance + filesystem.
jsonValue := front[m[2]:m[3]]
- // Translate the match indices, which are relative to `front`, into
- // the original content's coordinate space for splicing.
lineStart := frontStart + m[0]
lineEnd := frontStart + m[1]
- newBlock, err := transformMetadataJSON(jsonValue, strict)
+ env, primaryEnv, err := parseLegacyOpenclawJSON(jsonValue)
+ if err != nil {
+ return "", fmt.Errorf("parse legacy JSON: %w", err)
+ }
+ cliName, category, dirName, err := lookupProvenance(absPath, absRoot)
if err != nil {
- return "", fmt.Errorf("transform: %w", err)
+ return "", fmt.Errorf("provenance lookup: %w", err)
}
+ newBlock := emitMetadataBlock(cliName, buildModulePath(category, dirName, cliName), env, primaryEnv)
updated := append([]byte{}, content[:lineStart]...)
updated = append(updated, []byte(newBlock)...)
@@ -266,84 +287,31 @@ func frontmatterBounds(content []byte) (int, int, error) {
return 4, 4 + idx + 1, nil
}
-// installEntryJSON mirrors the legacy `install[]` entry shape we expect to
-// find in current SKILL.md files. We read kind, command, and bins; id and
-// label are silently ignored by encoding/json's unknown-field default since
-// the migration drops them anyway.
-type installEntryJSON struct {
- Kind string `json:"kind"`
- Command string `json:"command,omitempty"`
- Bins []string `json:"bins,omitempty"`
-}
-
-// openclawJSON is the embedded JSON shape we read out of the legacy
-// `metadata:` string.
+// openclawJSON is the subset of the legacy `metadata:` JSON we extract.
+// Only env and primaryEnv are used; the rest of the JSON (bins, install,
+// kind, command, id, label) is read by encoding/json but discarded —
+// module path comes from filesystem + provenance instead.
type openclawJSON struct {
Requires struct {
- Bins []string `json:"bins,omitempty"`
- Env []string `json:"env,omitempty"`
+ Env []string `json:"env,omitempty"`
} `json:"requires"`
- PrimaryEnv string `json:"primaryEnv,omitempty"`
- Install []installEntryJSON `json:"install,omitempty"`
+ PrimaryEnv string `json:"primaryEnv,omitempty"`
}
-// transformMetadataJSON parses a legacy metadata JSON string and emits the
-// canonical multi-line nested-YAML block. The block ends with a newline so
-// it slots in cleanly where the original single line lived.
-func transformMetadataJSON(jsonStr string, strict bool) (string, error) {
+// parseLegacyOpenclawJSON extracts env/primaryEnv from a legacy metadata
+// JSON string. Only malformed JSON is an error; structurally-unusual but
+// parseable JSON returns whatever env/primaryEnv it carries (or zero
+// values if absent). The migration is structurally robust without strict
+// validation: module path comes from provenance, env-absence emits a
+// no-auth block, and the loop never blocks on shape variation.
+func parseLegacyOpenclawJSON(jsonStr string) (env []string, primaryEnv string, err error) {
var meta struct {
Openclaw openclawJSON `json:"openclaw"`
}
if err := json.Unmarshal([]byte(jsonStr), &meta); err != nil {
- return "", fmt.Errorf("parse JSON: %w", err)
- }
- if len(meta.Openclaw.Requires.Bins) == 0 {
- return "", fmt.Errorf("openclaw.requires.bins missing")
- }
- if len(meta.Openclaw.Install) == 0 {
- return "", fmt.Errorf("openclaw.install[] missing")
- }
-
- // We only emit the first install entry's binary + module. Today every
- // printed CLI has exactly one install option (go install), so multi-entry
- // support is not yet exercised; revisit if multi-platform installs land.
- first := meta.Openclaw.Install[0]
- if strict {
- if first.Kind != "shell" {
- return "", fmt.Errorf("install[0].kind=%q (strict mode expects 'shell')", first.Kind)
- }
- if !strings.HasPrefix(first.Command, "go install ") {
- return "", fmt.Errorf("install[0].command does not start with 'go install '")
- }
- }
-
- module, err := deriveModule(first.Command)
- if err != nil {
- return "", fmt.Errorf("install[0]: %w", err)
+ return nil, "", fmt.Errorf("parse JSON: %w", err)
}
- bins := first.Bins
- if len(bins) == 0 {
- bins = meta.Openclaw.Requires.Bins
- }
- cliName := bins[0]
- return emitMetadataBlock(cliName, module, meta.Openclaw.Requires.Env, meta.Openclaw.PrimaryEnv), nil
-}
-
-// deriveModule strips "go install " prefix and "@latest" or "@<version>"
-// suffix from a command string and returns the bare module path.
-func deriveModule(command string) (string, error) {
- const prefix = "go install "
- if !strings.HasPrefix(command, prefix) {
- return "", fmt.Errorf("command does not start with 'go install '")
- }
- rest := strings.TrimPrefix(command, prefix)
- if at := strings.LastIndex(rest, "@"); at >= 0 {
- rest = rest[:at]
- }
- if rest == "" {
- return "", fmt.Errorf("module path empty after stripping prefix/version")
- }
- return rest, nil
+ return meta.Openclaw.Requires.Env, meta.Openclaw.PrimaryEnv, nil
}
// buildMetadataBlock is the synthesis-path emitter: derives the module
@@ -357,15 +325,23 @@ func deriveModule(command string) (string, error) {
// older binary-suffix convention (e.g., library/commerce/dominos-pp-cli/).
// Module path follows library/<category>/<dirName>/cmd/<cliName>.
func buildMetadataBlock(cliName, category, dirName string, env []string, primaryEnv string) string {
+ module := buildModulePath(category, dirName, cliName)
+ return emitMetadataBlock(cliName, module, env, primaryEnv)
+}
+
+// buildModulePath returns the canonical Go module path for an install
+// entry. dirName is the library directory basename (slug-only or
+// binary-suffix); cliName is the CLI binary name; both are needed because
+// the directory and binary names diverge for some CLIs.
+func buildModulePath(category, dirName, cliName string) string {
if category == "" {
category = "other"
}
if dirName == "" {
dirName = cliName
}
- module := "github.com/mvanhorn/printing-press-library/library/" + category +
+ return "github.com/mvanhorn/printing-press-library/library/" + category +
"/" + dirName + "/cmd/" + cliName
- return emitMetadataBlock(cliName, module, env, primaryEnv)
}
// emitMetadataBlock returns the canonical multi-line YAML metadata block.
@@ -411,78 +387,95 @@ func emitMetadataBlock(cliName, module string, env []string, primaryEnv string)
}
// lookupProvenance resolves cli_name, category, and the library directory
-// basename for synthesis.
+// basename for the SKILL.md at skillPath.
//
-// For library/*/<dir>/SKILL.md, reads the sibling .printing-press.json and
-// uses <dir> as the library directory basename.
-// For cli-skills/pp-<slug>/SKILL.md, scans library/*/*/.printing-press.json
-// for a directory matching <slug> or <slug>-pp-cli and uses that
-// directory's basename. The basename can differ from cli_name (slug-only
-// convention vs. older binary-suffix convention), so the module path must
-// be derived from the actual filesystem location.
+// Category and dirName always come from the file's filesystem position
+// (library/<category>/<dirName>/SKILL.md or cli-skills/pp-<slug>/SKILL.md
+// after pivoting through the registry). cli_name comes from the sibling
+// `.printing-press.json` because the binary name and the directory name
+// can diverge — older CLIs use library/<dir>/ where dir is binary-suffixed,
+// newer CLIs use slug-only directories. .printing-press.json's `category`
+// field is unreliable (often absent in older provenance files), so it's
+// not consulted; the filesystem path is the canonical truth for category.
func lookupProvenance(skillPath, absRoot string) (cliName, category, dirName string, err error) {
dir := filepath.Dir(skillPath)
- manifestPath := filepath.Join(dir, ".printing-press.json")
- data, readErr := os.ReadFile(manifestPath)
- if readErr == nil {
- cn, cat, perr := parseProvenance(data)
+ rel, err := filepath.Rel(absRoot, skillPath)
+ if err != nil {
+ return "", "", "", fmt.Errorf("relpath: %w", err)
+ }
+ parts := strings.Split(rel, string(filepath.Separator))
+
+ // Library file: library/<category>/<dir>/SKILL.md
+ if len(parts) == 4 && parts[0] == "library" && parts[3] == "SKILL.md" {
+ category = parts[1]
+ dirName = parts[2]
+ manifestPath := filepath.Join(dir, ".printing-press.json")
+ data, readErr := os.ReadFile(manifestPath)
+ if readErr != nil {
+ if errors.Is(readErr, fs.ErrNotExist) {
+ return "", "", "", fmt.Errorf("missing sibling .printing-press.json (need cli_name)")
+ }
+ return "", "", "", fmt.Errorf("read sibling provenance: %w", readErr)
+ }
+ cn, perr := parseProvenanceCLIName(data)
if perr != nil {
return "", "", "", perr
}
- return cn, cat, filepath.Base(dir), nil
- }
- // Only fall through to cli-skills mirror lookup when the sibling manifest
- // genuinely doesn't exist. Other read errors (permission denied, EIO, EISDIR)
- // would otherwise produce a misleading 'not a pp-* directory' error from the
- // fallthrough path.
- if !errors.Is(readErr, fs.ErrNotExist) {
- return "", "", "", fmt.Errorf("read sibling provenance: %w", readErr)
- }
- // cli-skills mirror: derive slug, scan library/.
- base := filepath.Base(dir)
- slug := strings.TrimPrefix(base, "pp-")
- if slug == base {
- return "", "", "", fmt.Errorf("no sibling .printing-press.json and not a pp-* directory")
- }
- candidates := []string{
- filepath.Join(absRoot, "library", "*", slug, ".printing-press.json"),
- filepath.Join(absRoot, "library", "*", slug+"-pp-cli", ".printing-press.json"),
- }
- for _, pattern := range candidates {
- // filepath.Glob errors on ErrBadPattern only; the patterns above are
- // static and known-good, so an error here means the OS layer is broken
- // rather than the input. Surface it instead of silently skipping.
- matches, globErr := filepath.Glob(pattern)
- if globErr != nil {
- return "", "", "", fmt.Errorf("glob %s: %w", pattern, globErr)
+ return cn, category, dirName, nil
+ }
+
+ // cli-skills mirror: cli-skills/pp-<slug>/SKILL.md. Pivot through the
+ // library tree to find the corresponding library entry whose category
+ // and directory basename are the canonical source.
+ if len(parts) == 3 && parts[0] == "cli-skills" && parts[2] == "SKILL.md" && strings.HasPrefix(parts[1], "pp-") {
+ slug := strings.TrimPrefix(parts[1], "pp-")
+ candidates := []string{
+ filepath.Join(absRoot, "library", "*", slug, ".printing-press.json"),
+ filepath.Join(absRoot, "library", "*", slug+"-pp-cli", ".printing-press.json"),
}
- for _, m := range matches {
- data, err := os.ReadFile(m)
- if err != nil {
- continue
+ for _, pattern := range candidates {
+ matches, globErr := filepath.Glob(pattern)
+ if globErr != nil {
+ return "", "", "", fmt.Errorf("glob %s: %w", pattern, globErr)
}
- cn, cat, perr := parseProvenance(data)
- if perr != nil {
- return "", "", "", perr
+ for _, m := range matches {
+ data, readErr := os.ReadFile(m)
+ if readErr != nil {
+ continue
+ }
+ cn, perr := parseProvenanceCLIName(data)
+ if perr != nil {
+ return "", "", "", perr
+ }
+ libDir := filepath.Dir(m)
+ libRel, err := filepath.Rel(absRoot, libDir)
+ if err != nil {
+ return "", "", "", fmt.Errorf("relpath libDir: %w", err)
+ }
+ libParts := strings.Split(libRel, string(filepath.Separator))
+ if len(libParts) != 3 || libParts[0] != "library" {
+ return "", "", "", fmt.Errorf("library entry at unexpected path: %s", libRel)
+ }
+ return cn, libParts[1], libParts[2], nil
}
- return cn, cat, filepath.Base(filepath.Dir(m)), nil
}
+ return "", "", "", fmt.Errorf("could not resolve provenance for cli-skills mirror pp-%s (no matching library/<cat>/%s or library/<cat>/%s-pp-cli)", slug, slug, slug)
}
- return "", "", "", fmt.Errorf("could not resolve provenance for cli-skills mirror %s", base)
+
+ return "", "", "", fmt.Errorf("path is neither library/<cat>/<dir>/SKILL.md nor cli-skills/pp-<slug>/SKILL.md: %s", rel)
}
-func parseProvenance(data []byte) (cliName, category string, err error) {
+func parseProvenanceCLIName(data []byte) (string, error) {
var pp struct {
- CLIName string `json:"cli_name"`
- Category string `json:"category"`
+ CLIName string `json:"cli_name"`
}
if err := json.Unmarshal(data, &pp); err != nil {
- return "", "", fmt.Errorf("parse .printing-press.json: %w", err)
+ return "", fmt.Errorf("parse .printing-press.json: %w", err)
}
if pp.CLIName == "" {
- return "", "", fmt.Errorf(".printing-press.json missing cli_name")
+ return "", fmt.Errorf(".printing-press.json missing cli_name")
}
- return pp.CLIName, pp.Category, nil
+ return pp.CLIName, nil
}
// atomicWrite writes data to path via tmp + rename. Preserves the original
diff --git a/tools/migrate-skill-metadata/main_test.go b/tools/migrate-skill-metadata/main_test.go
index 30e383d9..0ef7ac09 100644
--- a/tools/migrate-skill-metadata/main_test.go
+++ b/tools/migrate-skill-metadata/main_test.go
@@ -9,33 +9,6 @@ import (
// --- Unit tests for derivation helpers ---
-func TestDeriveModule(t *testing.T) {
- cases := []struct {
- name string
- input string
- want string
- wantErr bool
- }{
- {"latest tag", "go install github.com/x/y/cmd/z@latest", "github.com/x/y/cmd/z", false},
- {"explicit version", "go install github.com/x/y/cmd/z@v1.2.3", "github.com/x/y/cmd/z", false},
- {"no version tag", "go install github.com/x/y/cmd/z", "github.com/x/y/cmd/z", false},
- {"missing prefix", "github.com/x/y/cmd/z@latest", "", true},
- {"empty after strip", "go install @latest", "", true},
- {"path with @ in it", "go install github.com/x/y@email.com/cmd/z@latest", "github.com/x/y@email.com/cmd/z", false},
- }
- for _, tc := range cases {
- t.Run(tc.name, func(t *testing.T) {
- got, err := deriveModule(tc.input)
- if (err != nil) != tc.wantErr {
- t.Fatalf("err = %v, wantErr %v", err, tc.wantErr)
- }
- if got != tc.want {
- t.Errorf("got %q, want %q", got, tc.want)
- }
- })
- }
-}
-
func TestEmitMetadataBlock(t *testing.T) {
want := "metadata:\n" +
" openclaw:\n" +
@@ -104,99 +77,42 @@ func TestBuildMetadataBlockEmptyDirNameFallsBackToCliName(t *testing.T) {
}
}
-// --- transformMetadataJSON tests ---
-
-func TestTransformMetadataJSON_HappyPath(t *testing.T) {
- jsonStr := `{"openclaw":{"requires":{"bins":["dub-pp-cli"]},"install":[{"id":"go","kind":"shell","command":"go install github.com/mvanhorn/printing-press-library/library/other/dub/cmd/dub-pp-cli@latest","bins":["dub-pp-cli"],"label":"Install via go install"}]}}`
- got, err := transformMetadataJSON(jsonStr, true)
- if err != nil {
- t.Fatalf("unexpected error: %v", err)
- }
- if !strings.Contains(got, "kind: go") {
- t.Errorf("output missing kind: go; got:\n%s", got)
- }
- if !strings.Contains(got, "module: github.com/mvanhorn/printing-press-library/library/other/dub/cmd/dub-pp-cli") {
- t.Errorf("output missing expected module path; got:\n%s", got)
- }
- if strings.Contains(got, "kind: shell") {
- t.Errorf("output should not contain kind: shell; got:\n%s", got)
- }
- if strings.Contains(got, "command:") {
- t.Errorf("output should not contain command:; got:\n%s", got)
- }
- if strings.Contains(got, "id:") {
- t.Errorf("output should not contain id:; got:\n%s", got)
- }
- if strings.Contains(got, "label:") {
- t.Errorf("output should not contain label:; got:\n%s", got)
- }
- if strings.Contains(got, "@latest") {
- t.Errorf("module should have @latest stripped; got:\n%s", got)
- }
-}
+// --- parseLegacyOpenclawJSON tests ---
-func TestTransformMetadataJSON_WithAuthEnv(t *testing.T) {
- jsonStr := `{"openclaw":{"requires":{"bins":["kalshi-pp-cli"],"env":["KALSHI_TOKEN"]},"primaryEnv":"KALSHI_TOKEN","install":[{"id":"go","kind":"shell","command":"go install github.com/mvanhorn/printing-press-library/library/payments/kalshi/cmd/kalshi-pp-cli@latest","bins":["kalshi-pp-cli"],"label":"Install via go install"}]}}`
- got, err := transformMetadataJSON(jsonStr, true)
+func TestParseLegacyOpenclawJSON_NoAuth(t *testing.T) {
+ jsonStr := `{"openclaw":{"requires":{"bins":["dub-pp-cli"]},"install":[{"id":"go","kind":"shell","command":"go install github.com/x/y/cmd/dub-pp-cli@latest","bins":["dub-pp-cli"],"label":"Install via go install"}]}}`
+ env, primaryEnv, err := parseLegacyOpenclawJSON(jsonStr)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
- if !strings.Contains(got, "env:\n - KALSHI_TOKEN") {
- t.Errorf("output missing env list; got:\n%s", got)
+ if len(env) != 0 {
+ t.Errorf("expected no env, got %v", env)
}
- if !strings.Contains(got, "primaryEnv: KALSHI_TOKEN") {
- t.Errorf("output missing primaryEnv; got:\n%s", got)
+ if primaryEnv != "" {
+ t.Errorf("expected empty primaryEnv, got %q", primaryEnv)
}
}
-func TestTransformMetadataJSON_AgentCaptureBareName(t *testing.T) {
- jsonStr := `{"openclaw":{"requires":{"bins":["agent-capture"]},"install":[{"id":"go","kind":"shell","command":"go install github.com/mvanhorn/printing-press-library/library/developer-tools/agent-capture/cmd/agent-capture@latest","bins":["agent-capture"],"label":"Install via go install"}]}}`
- got, err := transformMetadataJSON(jsonStr, true)
+func TestParseLegacyOpenclawJSON_WithAuthEnv(t *testing.T) {
+ jsonStr := `{"openclaw":{"requires":{"bins":["kalshi-pp-cli"],"env":["KALSHI_TOKEN","KALSHI_KEY"]},"primaryEnv":"KALSHI_TOKEN","install":[{"id":"go","kind":"shell","command":"go install x@latest","bins":["kalshi-pp-cli"]}]}}`
+ env, primaryEnv, err := parseLegacyOpenclawJSON(jsonStr)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
- if !strings.Contains(got, "bins: [agent-capture]") {
- t.Errorf("output missing bare bins; got:\n%s", got)
+ want := []string{"KALSHI_TOKEN", "KALSHI_KEY"}
+ if len(env) != 2 || env[0] != want[0] || env[1] != want[1] {
+ t.Errorf("env mismatch: got %v want %v", env, want)
}
- if !strings.Contains(got, "/cmd/agent-capture\n") {
- t.Errorf("output module should end with /cmd/agent-capture; got:\n%s", got)
+ if primaryEnv != "KALSHI_TOKEN" {
+ t.Errorf("primaryEnv mismatch: got %q", primaryEnv)
}
}
-func TestTransformMetadataJSON_StrictRejectsBrewKind(t *testing.T) {
- jsonStr := `{"openclaw":{"requires":{"bins":["foo"]},"install":[{"kind":"brew","formula":"foo","bins":["foo"]}]}}`
- _, err := transformMetadataJSON(jsonStr, true)
- if err == nil {
- t.Fatalf("expected strict mode to reject kind: brew")
- }
- if !strings.Contains(err.Error(), "brew") {
- t.Errorf("error message should mention the rejected kind; got: %v", err)
- }
-}
-
-func TestTransformMetadataJSON_StrictRejectsNonGoInstallCommand(t *testing.T) {
- jsonStr := `{"openclaw":{"requires":{"bins":["foo"]},"install":[{"kind":"shell","command":"curl -L https://x/y | sh","bins":["foo"]}]}}`
- _, err := transformMetadataJSON(jsonStr, true)
- if err == nil {
- t.Fatalf("expected strict mode to reject non-go-install command")
- }
-}
-
-func TestTransformMetadataJSON_MalformedJSON(t *testing.T) {
- _, err := transformMetadataJSON(`{not valid json`, true)
+func TestParseLegacyOpenclawJSON_MalformedJSON(t *testing.T) {
+ _, _, err := parseLegacyOpenclawJSON(`{not valid json`)
if err == nil {
t.Fatalf("expected error on malformed JSON")
}
- if !strings.Contains(err.Error(), "parse JSON") {
- t.Errorf("error should mention JSON parse failure; got: %v", err)
- }
-}
-
-func TestTransformMetadataJSON_MissingBins(t *testing.T) {
- _, err := transformMetadataJSON(`{"openclaw":{"requires":{},"install":[{"kind":"shell","command":"go install x@latest","bins":["x"]}]}}`, true)
- if err == nil {
- t.Fatalf("expected error when requires.bins missing")
- }
}
// --- frontmatterBounds tests ---
@@ -236,6 +152,10 @@ func TestMigrateFile_LegacyJSONStringConversion(t *testing.T) {
if err := os.MkdirAll(dir, 0o755); err != nil {
t.Fatal(err)
}
+ if err := os.WriteFile(dir+"/.printing-press.json",
+ []byte(`{"cli_name":"dub-pp-cli","category":"other"}`), 0o644); err != nil {
+ t.Fatal(err)
+ }
skill := dir + "/SKILL.md"
original := `---
name: pp-dub
@@ -250,7 +170,7 @@ metadata: '{"openclaw":{"requires":{"bins":["dub-pp-cli"]},"install":[{"id":"go"
if err := os.WriteFile(skill, []byte(original), 0o644); err != nil {
t.Fatal(err)
}
- action, err := migrateFile(skill, root, true, false)
+ action, err := migrateFile(skill, root, false)
if err != nil {
t.Fatalf("migrateFile error: %v", err)
}
@@ -306,7 +226,7 @@ body
if err := os.WriteFile(skill, []byte(alreadyMigrated), 0o644); err != nil {
t.Fatal(err)
}
- action, err := migrateFile(skill, root, true, false)
+ action, err := migrateFile(skill, root, false)
if err != nil {
t.Fatalf("migrateFile error: %v", err)
}
@@ -325,6 +245,10 @@ func TestMigrateFile_DryRunDoesNotWrite(t *testing.T) {
if err := os.MkdirAll(dir, 0o755); err != nil {
t.Fatal(err)
}
+ if err := os.WriteFile(dir+"/.printing-press.json",
+ []byte(`{"cli_name":"dub-pp-cli","category":"other"}`), 0o644); err != nil {
+ t.Fatal(err)
+ }
skill := dir + "/SKILL.md"
original := `---
name: pp-dub
@@ -339,7 +263,7 @@ body
if err := os.WriteFile(skill, []byte(original), 0o644); err != nil {
t.Fatal(err)
}
- action, err := migrateFile(skill, root, true, true) // dryRun=true
+ action, err := migrateFile(skill, root, true) // dryRun=true
if err != nil {
t.Fatalf("migrateFile error: %v", err)
}
@@ -375,7 +299,7 @@ body
if err := os.WriteFile(skill, []byte(original), 0o644); err != nil {
t.Fatal(err)
}
- action, err := migrateFile(skill, root, true, false)
+ action, err := migrateFile(skill, root, false)
if err != nil {
t.Fatalf("migrateFile error: %v", err)
}
@@ -427,7 +351,7 @@ body
if err := os.WriteFile(skill, []byte(original), 0o644); err != nil {
t.Fatal(err)
}
- action, err := migrateFile(skill, root, true, false)
+ action, err := migrateFile(skill, root, false)
if err != nil {
t.Fatalf("migrateFile error: %v", err)
}
@@ -460,12 +384,55 @@ body
if err := os.WriteFile(skill, []byte(original), 0o644); err != nil {
t.Fatal(err)
}
- _, err := migrateFile(skill, root, true, false)
+ _, err := migrateFile(skill, root, false)
if err == nil {
t.Fatalf("expected error when synthesis cannot find provenance")
}
}
+// TestMigrateFile_RerouteCorrectsStalePath confirms that when the legacy
+// JSON's command field points at a stale path (e.g., a CLI moved between
+// categories without its SKILL.md being updated), the migrated module
+// reflects the file's actual filesystem location, not the stale path.
+func TestMigrateFile_RerouteCorrectsStalePath(t *testing.T) {
+ root := t.TempDir()
+ // CLI lives under library/payments/kalshi but the legacy JSON's
+ // command points at library/other/kalshi (stale category).
+ dir := filepath.Join(root, "library", "payments", "kalshi")
+ if err := os.MkdirAll(dir, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(dir+"/.printing-press.json",
+ []byte(`{"cli_name":"kalshi-pp-cli","category":"payments"}`), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ skill := dir + "/SKILL.md"
+ original := `---
+name: pp-kalshi
+description: "Kalshi CLI."
+argument-hint: "<command>"
+allowed-tools: "Read Bash"
+metadata: '{"openclaw":{"requires":{"bins":["kalshi-pp-cli"]},"install":[{"id":"go","kind":"shell","command":"go install github.com/mvanhorn/printing-press-library/library/other/kalshi/cmd/kalshi-pp-cli@latest","bins":["kalshi-pp-cli"],"label":"Install via go install"}]}}'
+---
+
+body
+`
+ if err := os.WriteFile(skill, []byte(original), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := migrateFile(skill, root, false); err != nil {
+ t.Fatalf("migrateFile error: %v", err)
+ }
+ got, _ := os.ReadFile(skill)
+ if !strings.Contains(string(got),
+ "module: github.com/mvanhorn/printing-press-library/library/payments/kalshi/cmd/kalshi-pp-cli") {
+ t.Errorf("rerouted module path missing; got:\n%s", string(got))
+ }
+ if strings.Contains(string(got), "library/other/kalshi") {
+ t.Errorf("stale path should have been replaced; got:\n%s", string(got))
+ }
+}
+
// TestRun_RejectsSymlinkEscapingRoot covers the path-traversal guard. A
// SKILL.md inside the root that's a symlink to a file outside the root
// must fail the EvalSymlinks-then-prefix check. This test fixture also
@@ -486,7 +453,7 @@ func TestRun_RejectsSymlinkEscapingRoot(t *testing.T) {
t.Skipf("symlink not supported on this filesystem: %v", err)
}
- report, err := run(root, true, true, false)
+ report, err := run(root, true, false)
if err != nil {
t.Fatalf("run returned error: %v", err)
}
@@ -513,6 +480,10 @@ func TestMigrateFile_BodyJSONShapeIsNotTouched(t *testing.T) {
if err := os.MkdirAll(dir, 0o755); err != nil {
t.Fatal(err)
}
+ if err := os.WriteFile(dir+"/.printing-press.json",
+ []byte(`{"cli_name":"x-pp-cli","category":"other"}`), 0o644); err != nil {
+ t.Fatal(err)
+ }
skill := dir + "/SKILL.md"
original := `---
name: pp-x
@@ -533,7 +504,7 @@ End.
if err := os.WriteFile(skill, []byte(original), 0o644); err != nil {
t.Fatal(err)
}
- if _, err := migrateFile(skill, root, true, false); err != nil {
+ if _, err := migrateFile(skill, root, false); err != nil {
t.Fatalf("migrateFile error: %v", err)
}
got, _ := os.ReadFile(skill)
← 4897c593 fix(cli): emit ClawHub-compliant nested YAML for SKILL.md me
·
back to Cli Printing Press
·
fix(cli): default live dogfood happy_path on mutators to --d 24e7e60f →