[object Object]

← back to Cli Printing Press

fix(publish): gate packaging on skill verification (#348)

47fe3393590de1cb71778de7b7748af3d6e1736c · 2026-04-27 15:07:50 -0700 · hnshah

Files touched

Diff

commit 47fe3393590de1cb71778de7b7748af3d6e1736c
Author: hnshah <hnshah@gmail.com>
Date:   Mon Apr 27 15:07:50 2026 -0700

    fix(publish): gate packaging on skill verification (#348)
---
 internal/cli/publish.go      |  22 ++++++-
 internal/cli/publish_test.go |  78 ++++++++++++++++++++++++-
 internal/cli/verify_skill.go | 136 +++++++++++++++++++++++++------------------
 3 files changed, 176 insertions(+), 60 deletions(-)

diff --git a/internal/cli/publish.go b/internal/cli/publish.go
index d70c3b88..6504f0d9 100644
--- a/internal/cli/publish.go
+++ b/internal/cli/publish.go
@@ -588,7 +588,15 @@ func runValidation(dir string) ValidateResult {
 		}
 	}
 
-	// 8. Manuscripts check (warn-only)
+	// 8. SKILL.md verification — fail if the agent-facing skill advertises
+	// commands, flags, or arguments the shipped CLI source does not provide.
+	skillCheck := checkVerifySkill(dir)
+	if !skillCheck.Passed {
+		allPassed = false
+	}
+	result.Checks = append(result.Checks, skillCheck)
+
+	// 9. Manuscripts check (warn-only)
 	// Try CLI name first (new convention), then API name, then fuzzy resolve
 	apiName := result.APIName
 	if apiName == "" {
@@ -617,6 +625,18 @@ func runValidation(dir string) ValidateResult {
 	return result
 }
 
+func checkVerifySkill(dir string) CheckResult {
+	run, err := runVerifySkillScript(dir, nil, false, false)
+	if err != nil {
+		errMsg := strings.TrimSpace(run.Stdout + "\n" + run.Stderr)
+		if errMsg == "" {
+			errMsg = err.Error()
+		}
+		return CheckResult{Name: "verify-skill", Passed: false, Error: errMsg}
+	}
+	return CheckResult{Name: "verify-skill", Passed: true}
+}
+
 func runGoCheck(dir string, args ...string) CheckResult {
 	name := "go " + args[0]
 	ctx, cancel := context.WithTimeout(context.Background(), goCommandTimeout)
diff --git a/internal/cli/publish_test.go b/internal/cli/publish_test.go
index 6f93b9f9..115330cc 100644
--- a/internal/cli/publish_test.go
+++ b/internal/cli/publish_test.go
@@ -150,7 +150,7 @@ func TestPublishValidateJSONHasAllChecks(t *testing.T) {
 	}
 
 	// All checks should be present (they may fail in test env, but must exist)
-	expectedChecks := []string{"manifest", "transcendence", "go mod tidy", "go vet", "go build", "--help", "--version", "manuscripts"}
+	expectedChecks := []string{"manifest", "transcendence", "go mod tidy", "go vet", "go build", "--help", "--version", "verify-skill", "manuscripts"}
 	for _, name := range expectedChecks {
 		assert.True(t, checkNames[name], "should have %q check", name)
 	}
@@ -313,6 +313,48 @@ func TestPublishPackageRejectsUnknownCategory(t *testing.T) {
 	assert.Contains(t, err.Error(), "--category must be one of:")
 }
 
+func TestPublishPackageFailsWhenSkillReferencesUnknownCommand(t *testing.T) {
+	home := setLibraryTestEnv(t)
+	cliDir := filepath.Join(home, "library", "test-pp-cli")
+	writePublishableTestCLI(t, cliDir)
+
+	// Regression guard: library CI caught SKILL.md references such as
+	// `wikipedia-pp-cli feed get-on-this-day` where the shipped CLI only had
+	// `feed`. publish package should fail locally before staging that PR.
+	skillPath := filepath.Join(cliDir, "SKILL.md")
+	f, err := os.OpenFile(skillPath, os.O_APPEND|os.O_WRONLY, 0o644)
+	require.NoError(t, err)
+	_, err = f.WriteString("\n```bash\ntest-pp-cli hallucinated-command --agent\n```\n")
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+
+	target := filepath.Join(t.TempDir(), "staging")
+	cmd := newPublishCmd()
+	cmd.SetArgs([]string{"package", "--dir", cliDir, "--category", "other", "--target", target, "--json"})
+
+	output, err := runWithCapturedStdout(t, cmd.Execute)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "validation failed")
+
+	var result ValidateResult
+	require.NoError(t, json.Unmarshal([]byte(output), &result))
+	assert.False(t, result.Passed)
+
+	var skillCheck *CheckResult
+	for i := range result.Checks {
+		if result.Checks[i].Name == "verify-skill" {
+			skillCheck = &result.Checks[i]
+			break
+		}
+	}
+	require.NotNil(t, skillCheck)
+	assert.False(t, skillCheck.Passed)
+	assert.Contains(t, skillCheck.Error, "unknown-command")
+
+	_, statErr := os.Stat(target)
+	assert.ErrorIs(t, statErr, os.ErrNotExist, "failed verification should not create staging target")
+}
+
 func TestPublishPackageDoesNotStageCompiledBinary(t *testing.T) {
 	home := setLibraryTestEnv(t)
 	cliDir := filepath.Join(home, "library", "test-pp-cli")
@@ -692,6 +734,13 @@ func writePublishableTestCLI(t *testing.T, dir string) {
 	require.NoError(t, os.WriteFile(filepath.Join(dir, "go.mod"), []byte(`module example.com/test-pp-cli
 
 go 1.24
+
+require github.com/spf13/cobra v1.10.2
+
+require (
+	github.com/inconshreveable/mousetrap v1.1.0 // indirect
+	github.com/spf13/pflag v1.0.9 // indirect
+)
 `), 0o644))
 	require.NoError(t, os.WriteFile(filepath.Join(dir, "cmd", "test-pp-cli", "main.go"), []byte(`package main
 
@@ -714,6 +763,33 @@ func main() {
 	fmt.Println("ok")
 }
 `), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "go.sum"), []byte(`github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
+github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
+github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
+github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
+github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU=
+github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4=
+github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY=
+github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
+go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+`), 0o644))
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "internal", "cli"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "internal", "cli", "root.go"), []byte(`package cli
+
+import "github.com/spf13/cobra"
+
+func newRootCmd() *cobra.Command {
+	cmd := &cobra.Command{Use: "test-pp-cli"}
+	cmd.AddCommand(newInsightCmd())
+	return cmd
+}
+
+func newInsightCmd() *cobra.Command {
+	return &cobra.Command{Use: "insight", Short: "Show test insight"}
+}
+`), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("# Test CLI\n\n## Command Reference\n\n- `test-pp-cli insight` — Show test insight\n\n## Usage\n\n```bash\ntest-pp-cli insight --agent\n```\n"), 0o644))
 
 	writeTestManifest(t, dir, pipeline.CLIManifest{
 		SchemaVersion: 1,
diff --git a/internal/cli/verify_skill.go b/internal/cli/verify_skill.go
index 288e9705..5ee96542 100644
--- a/internal/cli/verify_skill.go
+++ b/internal/cli/verify_skill.go
@@ -19,6 +19,74 @@ import (
 //go:embed verify_skill_bundled.py
 var verifySkillScript string
 
+type verifySkillRunResult struct {
+	Stdout   string
+	Stderr   string
+	ExitCode int
+}
+
+func runVerifySkillScript(dir string, only []string, asJSON bool, strict bool) (verifySkillRunResult, error) {
+	result := verifySkillRunResult{}
+
+	abs, err := filepath.Abs(dir)
+	if err != nil {
+		return result, fmt.Errorf("resolving --dir: %w", err)
+	}
+	if _, err := os.Stat(filepath.Join(abs, "SKILL.md")); err != nil {
+		result.ExitCode = ExitInputError
+		return result, &ExitError{Code: ExitInputError, Err: fmt.Errorf("no SKILL.md in %s", abs)}
+	}
+	if _, err := os.Stat(filepath.Join(abs, "internal", "cli")); err != nil {
+		result.ExitCode = ExitInputError
+		return result, &ExitError{Code: ExitInputError, Err: fmt.Errorf("no internal/cli/ in %s", abs)}
+	}
+
+	tmpFile, err := os.CreateTemp("", "verify-skill-*.py")
+	if err != nil {
+		return result, fmt.Errorf("creating temp file: %w", err)
+	}
+	defer func() { _ = os.Remove(tmpFile.Name()) }()
+	if _, err := tmpFile.WriteString(verifySkillScript); err != nil {
+		_ = tmpFile.Close()
+		return result, fmt.Errorf("writing temp file: %w", err)
+	}
+	if err := tmpFile.Close(); err != nil {
+		return result, fmt.Errorf("closing temp file: %w", err)
+	}
+
+	pyArgs := []string{tmpFile.Name(), "--dir", abs}
+	for _, o := range only {
+		pyArgs = append(pyArgs, "--only", o)
+	}
+	if asJSON {
+		pyArgs = append(pyArgs, "--json")
+	}
+	if strict {
+		pyArgs = append(pyArgs, "--strict")
+	}
+
+	py := exec.Command("python3", pyArgs...)
+	py.Stdin = os.Stdin
+	var stdout, stderr bytes.Buffer
+	py.Stdout = &stdout
+	py.Stderr = &stderr
+	runErr := py.Run()
+	result.Stdout = stdout.String()
+	result.Stderr = stderr.String()
+	if runErr != nil {
+		if exitErr, ok := runErr.(*exec.ExitError); ok {
+			result.ExitCode = exitErr.ExitCode()
+			return result, &ExitError{
+				Code:   exitErr.ExitCode(),
+				Err:    fmt.Errorf("SKILL verification failed"),
+				Silent: true,
+			}
+		}
+		return result, fmt.Errorf("running verifier: %w", runErr)
+	}
+	return result, nil
+}
+
 func newVerifySkillCmd() *cobra.Command {
 	var (
 		dir    string
@@ -32,11 +100,12 @@ func newVerifySkillCmd() *cobra.Command {
 		Short:         "Verify SKILL.md matches the shipped CLI source",
 		SilenceUsage:  true,
 		SilenceErrors: true,
-		Long: `Run three checks against a printed CLI's SKILL.md:
+		Long: `Run four checks against a printed CLI's SKILL.md:
 
   1. flag-names — every --flag referenced in SKILL.md is declared in internal/cli/*.go
   2. flag-commands — every --flag used on a specific command is declared on that command (or persistent)
   3. positional-args — positional args in bash recipes match the command's Use: field
+  4. unknown-command — every referenced command path maps to a cobra Use: declaration
 
 Fails when the SKILL advertises commands, flags, or arguments that the binary
 doesn't actually provide — which is how the recipe-goat "search --max-time"
@@ -45,7 +114,7 @@ CI check (scripts/verify-skill/verify_skill.py) via an embedded copy so no
 external script path is needed.
 
 Requires python3 on PATH (same dependency as the cookie-auth doctor check).`,
-		Example: `  # Run all three checks against a generated CLI
+		Example: `  # Run all checks against a generated CLI
   printing-press verify-skill --dir ./my-api-pp-cli
 
   # JSON output for programmatic consumption
@@ -57,73 +126,24 @@ Requires python3 on PATH (same dependency as the cookie-auth doctor check).`,
 			if dir == "" {
 				return &ExitError{Code: ExitInputError, Err: fmt.Errorf("--dir is required")}
 			}
-			abs, err := filepath.Abs(dir)
-			if err != nil {
-				return fmt.Errorf("resolving --dir: %w", err)
-			}
-			if _, err := os.Stat(filepath.Join(abs, "SKILL.md")); err != nil {
-				return &ExitError{Code: ExitInputError, Err: fmt.Errorf("no SKILL.md in %s", abs)}
-			}
-			if _, err := os.Stat(filepath.Join(abs, "internal", "cli")); err != nil {
-				return &ExitError{Code: ExitInputError, Err: fmt.Errorf("no internal/cli/ in %s", abs)}
-			}
-
-			tmpFile, err := os.CreateTemp("", "verify-skill-*.py")
-			if err != nil {
-				return fmt.Errorf("creating temp file: %w", err)
-			}
-			defer func() { _ = os.Remove(tmpFile.Name()) }()
-			if _, err := tmpFile.WriteString(verifySkillScript); err != nil {
-				_ = tmpFile.Close()
-				return fmt.Errorf("writing temp file: %w", err)
-			}
-			if err := tmpFile.Close(); err != nil {
-				return fmt.Errorf("closing temp file: %w", err)
-			}
-
-			pyArgs := []string{tmpFile.Name(), "--dir", abs}
-			for _, o := range only {
-				pyArgs = append(pyArgs, "--only", o)
-			}
-			if asJSON {
-				pyArgs = append(pyArgs, "--json")
-			}
-			if strict {
-				pyArgs = append(pyArgs, "--strict")
-			}
 
-			py := exec.Command("python3", pyArgs...)
-			py.Stdin = os.Stdin
-			var stdout, stderr bytes.Buffer
-			py.Stdout = &stdout
-			py.Stderr = &stderr
-			runErr := py.Run()
+			result, runErr := runVerifySkillScript(dir, only, asJSON, strict)
 			// Forward verifier output to the caller regardless of exit code.
-			if stdout.Len() > 0 {
-				fmt.Fprint(os.Stdout, stdout.String())
+			if result.Stdout != "" {
+				fmt.Fprint(os.Stdout, result.Stdout)
 			}
-			if stderr.Len() > 0 {
-				fmt.Fprint(os.Stderr, stderr.String())
+			if result.Stderr != "" {
+				fmt.Fprint(os.Stderr, result.Stderr)
 			}
 			if runErr != nil {
-				if exitErr, ok := runErr.(*exec.ExitError); ok {
-					// Propagate the verifier's exit code. 1 = findings, 2 = usage.
-					// Silent=true suppresses cobra's "Error: ..." prefix since
-					// the verifier already printed a human-readable report.
-					return &ExitError{
-						Code:   exitErr.ExitCode(),
-						Err:    fmt.Errorf("SKILL verification failed"),
-						Silent: true,
-					}
-				}
-				return fmt.Errorf("running verifier: %w", runErr)
+				return runErr
 			}
 			return nil
 		},
 	}
 
 	cmd.Flags().StringVar(&dir, "dir", "", "Path to the printed CLI directory (contains SKILL.md + internal/cli/)")
-	cmd.Flags().StringSliceVar(&only, "only", nil, "Run only the named check(s): flag-names, flag-commands, positional-args (repeatable)")
+	cmd.Flags().StringSliceVar(&only, "only", nil, "Run only the named check(s): flag-names, flag-commands, positional-args, unknown-command (repeatable)")
 	cmd.Flags().BoolVar(&asJSON, "json", false, "Output as JSON")
 	cmd.Flags().BoolVar(&strict, "strict", false, "Treat likely-false-positive findings as failures")
 

← 494abe48 feat(cli): add printing-press shipcheck umbrella + Phase 4 e  ·  back to Cli Printing Press  ·  fix(cli): machine improvements from hackernews retro #350 (# 0ed2fbd8 →