diff --git a/cmd/pilotctl/appstore_catalogue.go b/cmd/pilotctl/appstore_catalogue.go index 74ad9810..6c9c1c61 100644 --- a/cmd/pilotctl/appstore_catalogue.go +++ b/cmd/pilotctl/appstore_catalogue.go @@ -247,39 +247,10 @@ func cmdAppStoreCatalogue(_ []string) { fmt.Println("catalogue is empty") return } - fmt.Printf("Catalogue: %s (updated %s)\n\n", catalogueURL(), c.UpdatedAt) - fmt.Println("Installable apps:") for _, e := range c.Apps { - name := e.ID - if e.DisplayName != "" { - name = fmt.Sprintf("%s (%s)", e.DisplayName, e.ID) - } - fmt.Printf("\n %s v%s\n", name, e.Version) - // Teaser line: vendor · categories · license · size — only the - // parts a v2 entry actually carries. v1 entries skip it entirely. - var bits []string - if e.Vendor != "" { - bits = append(bits, e.Vendor) - } - if len(e.Categories) > 0 { - bits = append(bits, strings.Join(e.Categories, ", ")) - } - if e.License != "" { - bits = append(bits, e.License) - } - if e.BundleSize > 0 { - bits = append(bits, formatBytes(uint64(e.BundleSize))) - } - if len(bits) > 0 { - fmt.Printf(" %s\n", strings.Join(bits, " · ")) - } - fmt.Printf(" %s\n", e.Description) - // Point at the new detail view when extended metadata is published. - if e.MetadataURL != "" { - fmt.Printf(" view: pilotctl appstore view %s\n", e.ID) - } - fmt.Printf(" install: pilotctl appstore install %s\n", e.ID) + fmt.Printf("%-40s %s\n", e.ID, e.Description) } + fmt.Println("\nRun 'pilotctl appstore view ' for full details.") } // installSource tags how a bundle reached the install command. diff --git a/cmd/pilotctl/main.go b/cmd/pilotctl/main.go index 193f8abe..db1d1314 100644 --- a/cmd/pilotctl/main.go +++ b/cmd/pilotctl/main.go @@ -1219,6 +1219,8 @@ Reliability caveats (current implementation): // falls through pilotctl's per-command help intercept and // prints "No specific help" — confusing for an RC-shipped CLI. "appstore": AppStoreHelpText, + + "review": reviewHelpText, } // printCommandHelp prints the help text for a command and exits. @@ -1422,6 +1424,10 @@ dispatch: cmdAppStore(cmdArgs) return + case "review": + cmdReview(cmdArgs) + return + // Bootstrap case "init": cmdInit(cmdArgs) diff --git a/cmd/pilotctl/review.go b/cmd/pilotctl/review.go new file mode 100644 index 00000000..4d3df770 --- /dev/null +++ b/cmd/pilotctl/review.go @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import ( + "fmt" + "os" + "strconv" + "strings" +) + +// reviewHelpText is the canonical help block for `pilotctl review`. +// Registered in commandHelp in main.go so `pilotctl review --help` works. +const reviewHelpText = `Usage: pilotctl review [--rating N] [--text "..."] + +Submit a review for Pilot itself or for an installed app. + +Arguments: + pilot review the Pilot Protocol itself + review a specific app (e.g. io.pilot.cosift) + +Flags: + --rating N integer rating 1–5 (optional) + --text "..." review text (optional) + --help show this help + +Examples: + pilotctl review pilot + pilotctl review pilot --rating 5 --text "Works great" + pilotctl review io.pilot.cosift --rating 4 + pilotctl review io.pilot.cosift --text "Very useful app" + +Note: telemetry routing is not yet enabled (PILOT-411). This command +validates input and confirms receipt; no data is transmitted. +` + +// cmdReview handles `pilotctl review [--rating N] [--text "..."]`. +// +// Validation: +// - subject is required and must be non-empty +// - --rating, when present, must be an integer in [1, 5] +// - --text is free-form (no constraint) +// +// On valid input: prints a confirmation line and exits 0. +// On invalid input: prints an error + usage hint to stderr, exits 1. +// +// Telemetry routing (PILOT-411) is not yet implemented; this is a +// validation + stub only. +func cmdReview(args []string) { + flags, pos := parseFlags(args) + + if len(pos) == 0 { + if jsonOutput { + fatalCode("invalid_argument", + "subject is required: 'pilot' or an app-id (e.g. io.pilot.cosift)") + } + fmt.Fprintf(os.Stderr, "error: subject is required\nhint: usage: pilotctl review [--rating N] [--text \"...\"]\n") + os.Exit(1) + } + + subject := pos[0] + if strings.TrimSpace(subject) == "" { + fatalCode("invalid_argument", "subject must not be empty") + } + + // Validate --rating when provided. + var rating int + hasRating := false + if rStr, ok := flags["rating"]; ok { + hasRating = true + n, err := strconv.Atoi(rStr) + if err != nil { + if jsonOutput { + fatalCode("invalid_argument", + "--rating must be an integer between 1 and 5, got %q", rStr) + } + fmt.Fprintf(os.Stderr, + "error: --rating must be an integer between 1 and 5, got %q\nhint: usage: pilotctl review [--rating N] [--text \"...\"]\n", + rStr) + os.Exit(1) + } + if n < 1 || n > 5 { + if jsonOutput { + fatalCode("invalid_argument", + "--rating must be between 1 and 5, got %d", n) + } + fmt.Fprintf(os.Stderr, + "error: --rating must be between 1 and 5, got %d\nhint: usage: pilotctl review [--rating N] [--text \"...\"]\n", + n) + os.Exit(1) + } + rating = n + } + + reviewText := flagString(flags, "text", "") + + if jsonOutput { + out := map[string]interface{}{ + "subject": subject, + "submitted": true, + } + if hasRating { + out["rating"] = rating + } + if reviewText != "" { + out["text"] = reviewText + } + outputOK(out) + return + } + + fmt.Printf("Review submitted for %s. Thank you!\n", subject) +} diff --git a/cmd/pilotctl/zz_appstore_cmds_test.go b/cmd/pilotctl/zz_appstore_cmds_test.go index 79d41d10..e59b314a 100644 --- a/cmd/pilotctl/zz_appstore_cmds_test.go +++ b/cmd/pilotctl/zz_appstore_cmds_test.go @@ -880,3 +880,90 @@ func TestCmdAppStoreDispatcher(t *testing.T) { // no-args branch prints help to stderr _ = captureStderr(t, func() { cmdAppStore(nil) }) } + +// TestCmdAppStoreCatalogueTextOneLinePerApp asserts that text-mode output +// prints exactly one line per app in the form " " (PILOT-405). +func TestCmdAppStoreCatalogueTextOneLinePerApp(t *testing.T) { + stageCatalogue(t, `{"version":2,"updated_at":"2026-06-17","apps":[ + {"id":"io.pilot.wallet","version":"1.0.0","description":"Manages x402 payment credentials","bundle_url":"https://x/a.tgz","bundle_sha256":"abc"}, + {"id":"io.pilot.cosift","version":"0.2.0","description":"Web search and answer agent","bundle_url":"https://x/b.tgz","bundle_sha256":"def"} + ]}`) + + prev := jsonOutput + defer func() { jsonOutput = prev }() + jsonOutput = false + + out := captureStdout(t, func() { cmdAppStoreCatalogue(nil) }) + + // Each app must appear as a single line containing both id and description. + for _, want := range []string{ + "io.pilot.wallet", + "Manages x402 payment credentials", + "io.pilot.cosift", + "Web search and answer agent", + } { + if !strings.Contains(out, want) { + t.Errorf("missing %q in catalogue output:\n%s", want, out) + } + } + + // Both apps must appear on their own lines (one line each, not merged). + lines := strings.Split(strings.TrimSpace(out), "\n") + var appLines []string + for _, l := range lines { + if strings.Contains(l, "io.pilot.") { + appLines = append(appLines, l) + } + } + if len(appLines) != 2 { + t.Errorf("expected 2 app lines, got %d:\n%s", len(appLines), out) + } + // Each app line must contain the id and the description on the same line. + if !strings.Contains(appLines[0], "io.pilot.wallet") || !strings.Contains(appLines[0], "Manages x402") { + t.Errorf("first app line wrong: %q", appLines[0]) + } + if !strings.Contains(appLines[1], "io.pilot.cosift") || !strings.Contains(appLines[1], "Web search") { + t.Errorf("second app line wrong: %q", appLines[1]) + } +} + +// TestCmdAppStoreCatalogueTextViewPointerHint asserts the view-pointer hint +// appears at the end of text-mode catalogue output (PILOT-405). +func TestCmdAppStoreCatalogueTextViewPointerHint(t *testing.T) { + stageCatalogue(t, `{"version":1,"updated_at":"2026-06-17","apps":[ + {"id":"io.pilot.wallet","version":"1.0.0","description":"Manages x402 payment credentials","bundle_url":"https://x/a.tgz","bundle_sha256":"abc"} + ]}`) + + prev := jsonOutput + defer func() { jsonOutput = prev }() + jsonOutput = false + + out := captureStdout(t, func() { cmdAppStoreCatalogue(nil) }) + + const hint = "Run 'pilotctl appstore view ' for full details." + if !strings.Contains(out, hint) { + t.Errorf("view-pointer hint missing from catalogue output:\n%s", out) + } +} + +// TestCmdAppStoreCatalogueJSONUnchanged confirms --json output is unchanged: +// it still emits a JSON array of catalogue entries (PILOT-405). +func TestCmdAppStoreCatalogueJSONUnchanged(t *testing.T) { + stageCatalogue(t, `{"version":1,"updated_at":"2026-06-17","apps":[ + {"id":"io.pilot.wallet","version":"1.0.0","description":"Manages x402 payment credentials","bundle_url":"https://x/a.tgz","bundle_sha256":"abc"} + ]}`) + + prev := jsonOutput + defer func() { jsonOutput = prev }() + jsonOutput = true + + out := captureStdout(t, func() { cmdAppStoreCatalogue(nil) }) + + var apps []catalogueEntry + if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &apps); err != nil { + t.Fatalf("json output is not a valid array: %v\n%s", err, out) + } + if len(apps) != 1 || apps[0].ID != "io.pilot.wallet" { + t.Errorf("unexpected apps: %+v", apps) + } +} diff --git a/cmd/pilotctl/zz_review_test.go b/cmd/pilotctl/zz_review_test.go new file mode 100644 index 00000000..a41f13e9 --- /dev/null +++ b/cmd/pilotctl/zz_review_test.go @@ -0,0 +1,173 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import ( + "encoding/json" + "strings" + "testing" +) + +// --- unit tests: cmdReview (call directly, no os.Exit paths) --- + +// TestReviewValidSubjectNoFlags: bare subject, no optional flags → exit 0 + confirmation. +func TestReviewValidSubjectNoFlags(t *testing.T) { + t.Parallel() + stdout, _, code := runCLI(t, []string{"review", "io.pilot.cosift"}, nil) + if code != 0 { + t.Errorf("exit=%d, want 0", code) + } + if !strings.Contains(stdout, "io.pilot.cosift") { + t.Errorf("confirmation missing subject: %q", stdout) + } + if !strings.Contains(strings.ToLower(stdout), "thank") { + t.Errorf("confirmation missing thank-you: %q", stdout) + } +} + +// TestReviewSubjectPilot: reserved subject "pilot" is valid. +func TestReviewSubjectPilot(t *testing.T) { + t.Parallel() + stdout, _, code := runCLI(t, []string{"review", "pilot"}, nil) + if code != 0 { + t.Errorf("exit=%d, want 0", code) + } + if !strings.Contains(stdout, "pilot") { + t.Errorf("confirmation missing 'pilot': %q", stdout) + } +} + +// TestReviewWithRating: valid rating (1–5) is accepted. +func TestReviewWithRating(t *testing.T) { + t.Parallel() + for _, rating := range []string{"1", "3", "5"} { + rating := rating + t.Run("rating="+rating, func(t *testing.T) { + t.Parallel() + stdout, _, code := runCLI(t, []string{"review", "pilot", "--rating", rating}, nil) + if code != 0 { + t.Errorf("rating=%s: exit=%d, want 0", rating, code) + } + if !strings.Contains(stdout, "pilot") { + t.Errorf("rating=%s: missing subject in output: %q", rating, stdout) + } + }) + } +} + +// TestReviewWithRatingAndText: both optional flags together. +func TestReviewWithRatingAndText(t *testing.T) { + t.Parallel() + stdout, _, code := runCLI(t, []string{ + "review", "io.pilot.cosift", + "--rating", "4", + "--text", "Very useful", + }, nil) + if code != 0 { + t.Errorf("exit=%d, want 0", code) + } + if !strings.Contains(stdout, "io.pilot.cosift") { + t.Errorf("missing subject: %q", stdout) + } +} + +// TestReviewJSONOutput: --json wraps the result in the standard envelope. +func TestReviewJSONOutput(t *testing.T) { + t.Parallel() + stdout, _, code := runCLI(t, []string{ + "--json", "review", "pilot", "--rating", "5", "--text", "great", + }, nil) + if code != 0 { + t.Errorf("exit=%d, want 0", code) + } + var env map[string]interface{} + if err := json.Unmarshal([]byte(stdout), &env); err != nil { + t.Fatalf("json parse: %v (output: %q)", err, stdout) + } + if env["status"] != "ok" { + t.Errorf("status=%v, want ok", env["status"]) + } + data, ok := env["data"].(map[string]interface{}) + if !ok { + t.Fatalf("data field missing or wrong type: %v", env["data"]) + } + if data["subject"] != "pilot" { + t.Errorf("subject=%v, want pilot", data["subject"]) + } + if data["submitted"] != true { + t.Errorf("submitted=%v, want true", data["submitted"]) + } + if rating, ok := data["rating"].(float64); !ok || rating != 5 { + t.Errorf("rating=%v, want 5", data["rating"]) + } + if data["text"] != "great" { + t.Errorf("text=%v, want 'great'", data["text"]) + } +} + +// --- invalid input tests (use runCLI so os.Exit is safe) --- + +// TestReviewMissingSubject: no positional arg → exit non-zero with usage hint. +func TestReviewMissingSubject(t *testing.T) { + t.Parallel() + _, stderr, code := runCLI(t, []string{"review"}, nil) + if code == 0 { + t.Error("expected non-zero exit when subject is missing") + } + if !strings.Contains(stderr, "subject") && !strings.Contains(stderr, "required") && + !strings.Contains(stderr, "usage") && !strings.Contains(stderr, "pilot") { + t.Errorf("expected usage hint in stderr, got: %q", stderr) + } +} + +// TestReviewRatingZero: --rating 0 is out of range. +func TestReviewRatingZero(t *testing.T) { + t.Parallel() + _, stderr, code := runCLI(t, []string{"review", "pilot", "--rating", "0"}, nil) + if code == 0 { + t.Error("expected non-zero exit for --rating 0") + } + if !strings.Contains(stderr, "1") || !strings.Contains(stderr, "5") { + t.Errorf("expected range hint (1–5) in stderr, got: %q", stderr) + } +} + +// TestReviewRatingSix: --rating 6 is out of range. +func TestReviewRatingSix(t *testing.T) { + t.Parallel() + _, stderr, code := runCLI(t, []string{"review", "pilot", "--rating", "6"}, nil) + if code == 0 { + t.Error("expected non-zero exit for --rating 6") + } + if !strings.Contains(stderr, "1") || !strings.Contains(stderr, "5") { + t.Errorf("expected range hint (1–5) in stderr, got: %q", stderr) + } +} + +// TestReviewRatingNonInteger: --rating xyz is not a number. +func TestReviewRatingNonInteger(t *testing.T) { + t.Parallel() + _, stderr, code := runCLI(t, []string{"review", "pilot", "--rating", "xyz"}, nil) + if code == 0 { + t.Error("expected non-zero exit for non-integer --rating") + } + if !strings.Contains(stderr, "integer") && !strings.Contains(stderr, "rating") { + t.Errorf("expected error mentioning rating, got: %q", stderr) + } +} + +// TestReviewHelpFlag: --help exits 0 and prints usage. +func TestReviewHelpFlag(t *testing.T) { + t.Parallel() + stdout, stderr, code := runCLI(t, []string{"review", "--help"}, nil) + if code != 0 { + t.Errorf("--help should exit 0, got %d", code) + } + full := stdout + stderr + if !strings.Contains(full, "review") { + t.Errorf("help text missing 'review': %s", full) + } + if !strings.Contains(full, "--rating") { + t.Errorf("help text missing --rating: %s", full) + } +}