From 0105acbeb8b5bbbbdb8930a55914e0741df1be35 Mon Sep 17 00:00:00 2001 From: Ory Medina Date: Wed, 5 Aug 2026 17:31:29 -0600 Subject: [PATCH] [TOL-2358] Add feedback command --- README.md | 13 ++- internal/cli/feedback.go | 149 +++++++++++++++++++++++++ internal/cli/feedback_test.go | 126 +++++++++++++++++++++ internal/cli/index.go | 1 + internal/client/tollbit/client.go | 35 ++++++ internal/client/tollbit/client_test.go | 81 ++++++++++++++ skill/tollbit-cli/SKILL.md | 3 +- 7 files changed, 406 insertions(+), 2 deletions(-) create mode 100644 internal/cli/feedback.go create mode 100644 internal/cli/feedback_test.go diff --git a/README.md b/README.md index ca1ab8e..a935db6 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ See [CONTRIBUTING.md](CONTRIBUTING.md) for local development and release instruc - Run **`tollbit guide`** for orientation, then **`tollbit guide --install `** to persist the bundled skill. The guide is the full automation contract (exit codes, streams, non-interactive fetch). - Typical flow: **`search`** → **`content pricing`** → **`content fetch`**. The CLI prompts for authentication when a token is required. - Optionally set an agent profile with **`tollbit auth set --name `** (or `TOLLBIT_AGENT_DEFAULT_NAME`) / **`--user-agent`**. -- Prefer **`--json`** on **`search`**, **`content pricing`**, **`content fetch`**, and **`auth status`**. Exit codes: `0` success, `1` runtime, `2` usage; stdout is data-only (hints and errors on stderr). +- Prefer **`--json`** on **`search`**, **`content pricing`**, **`content fetch`**, **`feedback`**, and **`auth status`**. Exit codes: `0` success, `1` runtime, `2` usage; stdout is data-only (hints and errors on stderr). ## What the CLI can do @@ -75,6 +75,7 @@ See [CONTRIBUTING.md](CONTRIBUTING.md) for local development and release instruc |--------|---------| | **`search "query"`** | Search publisher content via the gateway API. | | **`content pricing/fetch`** | Price and fetch licensed publisher content. | +| **`feedback "message"`** | Submit CLI / agent feedback (requires OBO). | | **`auth login/logout/status/set`** | Agent profile and OAuth authorization token (also run automatically when needed). | | **`guide`** | Print the agent guide; optionally install bundled skill markdown. | | **`version`** | Print the CLI version string. | @@ -107,6 +108,16 @@ tollbit content fetch https://example.com/article --confirm --json --rate-index **Every fetch charges money.** Pricing is shown and you must confirm unless you pass `--confirm` (automation still incurs cost). Use `--toDisk=` to save fetched content locally. When no user agent is configured, the org default `-tbcli-` agent is used. Set a registered user agent with `auth set --user-agent` or `--user-agent` on the fetch command. +### Feedback + +Submit product feedback (requires an OBO-linked agent session): + +```bash +tollbit feedback "search ranking felt off for this query" +tollbit feedback "great fetch UX" --rating 5 --category content +tollbit feedback "auth hung" --rating 2 --category auth --metadata source=cli --json +``` + ### Auth Auth runs automatically when a command needs a token. Use these commands to inspect or manage the profile explicitly: diff --git a/internal/cli/feedback.go b/internal/cli/feedback.go new file mode 100644 index 0000000..602d479 --- /dev/null +++ b/internal/cli/feedback.go @@ -0,0 +1,149 @@ +package cli + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + "github.com/tollbit/cli/internal/app" + "github.com/tollbit/cli/internal/client/tollbit" + "github.com/tollbit/cli/internal/credentials/agenttoken" + "github.com/tollbit/cli/internal/tokens/agent" +) + +const feedbackLongHelp = `Submit feedback about the TollBit CLI or agent experience. + +Requires an authenticated agent with on-behalf-of (OBO) consent. Feedback is +accepted asynchronously and delivered to Tollbit (Slack + spreadsheet).` + +type feedbackOptions struct { + rating int + category string + metadata []string + userAgent string + asJSON bool +} + +func NewFeedbackCommand(factory app.Factory) *cobra.Command { + var opts feedbackOptions + + cmd := &cobra.Command{ + Use: `feedback "message"`, + Short: "Submit feedback to Tollbit", + Long: feedbackLongHelp, + Args: func(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + return UsageError(`feedback requires exactly one message argument`) + } + if strings.TrimSpace(args[0]) == "" { + return UsageError("feedback message must not be empty") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + return runFeedback(cmd, factory, opts, strings.Trim(args[0], `"'`)) + }, + } + + cmd.Flags().IntVar(&opts.rating, "rating", 0, "optional rating from 1 (worst) to 5 (best)") + cmd.Flags().StringVar(&opts.category, "category", "", "optional category label (e.g. search, auth, content)") + cmd.Flags().StringArrayVar(&opts.metadata, "metadata", nil, "optional key=value context (repeatable)") + cmd.Flags().StringVar(&opts.userAgent, "user-agent", "", "user agent for request") + cmd.Flags().BoolVar(&opts.asJSON, "json", false, "emit raw JSON response") + + return cmd +} + +func runFeedback(cmd *cobra.Command, factory app.Factory, opts feedbackOptions, message string) error { + message = strings.TrimSpace(message) + if message == "" { + return UsageError("feedback message must not be empty") + } + + req := tollbit.SubmitFeedbackRequest{ + Message: message, + Category: strings.TrimSpace(opts.category), + } + if cmd.Flags().Changed("rating") { + if opts.rating < 1 || opts.rating > 5 { + return UsageError("feedback --rating must be between 1 and 5") + } + rating := opts.rating + req.Rating = &rating + } + metadata, err := parseMetadataFlags(opts.metadata) + if err != nil { + return err + } + req.Metadata = metadata + + app, err := appForCommand(factory, cmd) + if err != nil { + return RuntimeError(err) + } + credentials, err := app.Credentials() + if err != nil { + return RuntimeError(err) + } + tollbitClient, err := app.Tollbit() + if err != nil { + return RuntimeError(err) + } + + identityOpts := agenttoken.ResolveIdentityOptions{ + UserAgent: flagChangedStr(cmd, "user-agent"), + } + identity, err := credentials.ResolveIdentity(cmd.Context(), identityOpts) + if err != nil { + return RuntimeError(fmt.Errorf("error resolving identity: %w", err)) + } + + var resp tollbit.SubmitFeedbackResponse + if app.Config().Auth.RetryOnOBORequired { + resp, err = agenttoken.WithOBORetry(cmd, credentials, identity, func(token agent.Token) (tollbit.SubmitFeedbackResponse, error) { + return tollbitClient.SubmitFeedback(cmd.Context(), req, token) + }) + } else { + token, tokenErr := credentials.GetAgentToken(cmd, identity) + if tokenErr != nil { + return RuntimeError(fmt.Errorf("error fetching agent token: %w", tokenErr)) + } + resp, err = tollbitClient.SubmitFeedback(cmd.Context(), req, token) + } + if err != nil { + return RuntimeError(fmt.Errorf("error submitting feedback: %w", err)) + } + + if opts.asJSON { + return RuntimeError(writeJSON(cmd.OutOrStdout(), resp)) + } + if resp.Accepted { + fmt.Fprintln(cmd.OutOrStdout(), "Feedback accepted.") + } else { + fmt.Fprintln(cmd.OutOrStdout(), "Feedback was not accepted.") + } + return nil +} + +func parseMetadataFlags(values []string) (map[string]string, error) { + if len(values) == 0 { + return nil, nil + } + out := make(map[string]string, len(values)) + for _, raw := range values { + raw = strings.TrimSpace(raw) + if raw == "" { + continue + } + key, value, ok := strings.Cut(raw, "=") + key = strings.TrimSpace(key) + if !ok || key == "" { + return nil, UsageError("feedback --metadata must be key=value, got %q", raw) + } + out[key] = strings.TrimSpace(value) + } + if len(out) == 0 { + return nil, nil + } + return out, nil +} diff --git a/internal/cli/feedback_test.go b/internal/cli/feedback_test.go new file mode 100644 index 0000000..0001049 --- /dev/null +++ b/internal/cli/feedback_test.go @@ -0,0 +1,126 @@ +package cli + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/tollbit/cli/internal/client/tollbit" +) + +func TestParseMetadataFlags(t *testing.T) { + meta, err := parseMetadataFlags([]string{"source=cli", " feature = search ", "empty="}) + if err != nil { + t.Fatal(err) + } + if meta["source"] != "cli" || meta["feature"] != "search" || meta["empty"] != "" { + t.Fatalf("unexpected metadata: %#v", meta) + } + + _, err = parseMetadataFlags([]string{"no-equals"}) + if err == nil || !strings.Contains(err.Error(), "key=value") { + t.Fatalf("expected key=value usage error, got %v", err) + } +} + +func TestRunFeedbackAccepted(t *testing.T) { + token := testAgentJWT(t) + authSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.RequestURI() != "/agent/v1/tokens/identity" { + t.Fatalf("unexpected auth request: %s %s", r.Method, r.URL.RequestURI()) + } + _ = json.NewEncoder(w).Encode(map[string]string{"token": token}) + })) + defer authSrv.Close() + + gatewaySrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/agents/v1/feedback" { + t.Fatalf("unexpected gateway request: %s %s", r.Method, r.URL.String()) + } + if r.Header.Get("Authorization") != "Bearer "+token { + t.Fatalf("unexpected authorization header: %q", r.Header.Get("Authorization")) + } + var body tollbit.SubmitFeedbackRequest + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatal(err) + } + if body.Message != "CLI smoke" { + t.Fatalf("unexpected message: %q", body.Message) + } + if body.Rating == nil || *body.Rating != 5 { + t.Fatalf("unexpected rating: %#v", body.Rating) + } + if body.Category != "deploy" { + t.Fatalf("unexpected category: %q", body.Category) + } + if body.Metadata["source"] != "cli-test" { + t.Fatalf("unexpected metadata: %#v", body.Metadata) + } + w.WriteHeader(http.StatusAccepted) + _ = json.NewEncoder(w).Encode(tollbit.SubmitFeedbackResponse{Accepted: true}) + })) + defer gatewaySrv.Close() + + t.Setenv(testAuthBaseURLEnvVar, authSrv.URL) + t.Setenv(testGatewayBaseURLEnvVar, gatewaySrv.URL) + t.Setenv(testCredentialsStorageDirEnvVar, t.TempDir()) + var stdout, stderr bytes.Buffer + code := executeTestCommand([]string{ + "feedback", "CLI smoke", + "--rating", "5", + "--category", "deploy", + "--metadata", "source=cli-test", + }, nil, &stdout, &stderr) + if code != 0 { + t.Fatalf("expected exit code 0, got %d stderr=%q", code, stderr.String()) + } + if want := "Feedback accepted."; !strings.Contains(stdout.String(), want) { + t.Fatalf("expected stdout to contain %q, got %q", want, stdout.String()) + } +} + +func TestRunFeedbackJSON(t *testing.T) { + token := testAgentJWT(t) + authSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]string{"token": token}) + })) + defer authSrv.Close() + + gatewaySrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusAccepted) + _ = json.NewEncoder(w).Encode(tollbit.SubmitFeedbackResponse{Accepted: true}) + })) + defer gatewaySrv.Close() + + t.Setenv(testAuthBaseURLEnvVar, authSrv.URL) + t.Setenv(testGatewayBaseURLEnvVar, gatewaySrv.URL) + t.Setenv(testCredentialsStorageDirEnvVar, t.TempDir()) + var stdout, stderr bytes.Buffer + code := executeTestCommand([]string{"feedback", "json please", "--json"}, nil, &stdout, &stderr) + if code != 0 { + t.Fatalf("expected exit code 0, got %d stderr=%q", code, stderr.String()) + } + var resp tollbit.SubmitFeedbackResponse + if err := json.NewDecoder(&stdout).Decode(&resp); err != nil { + t.Fatal(err) + } + if !resp.Accepted { + t.Fatal("expected accepted true") + } +} + +func TestRunFeedbackUsageErrors(t *testing.T) { + var stdout, stderr bytes.Buffer + code := executeTestCommand([]string{"feedback"}, nil, &stdout, &stderr) + if code != 2 { + t.Fatalf("expected usage exit 2, got %d stderr=%q", code, stderr.String()) + } + + code = executeTestCommand([]string{"feedback", "hi", "--rating", "9"}, nil, &stdout, &stderr) + if code != 2 { + t.Fatalf("expected usage exit 2 for bad rating, got %d stderr=%q", code, stderr.String()) + } +} diff --git a/internal/cli/index.go b/internal/cli/index.go index 8397911..ef8d5b9 100644 --- a/internal/cli/index.go +++ b/internal/cli/index.go @@ -10,6 +10,7 @@ func NewCommandTree(factory app.Factory) *cobra.Command { rootCmd.AddCommand( NewAuthCommand(factory), NewContentCommand(factory), + NewFeedbackCommand(factory), NewSearchCommand(factory), NewRuntimeCommand(factory), NewGuideCommand(factory), diff --git a/internal/client/tollbit/client.go b/internal/client/tollbit/client.go index 55f8dfd..55c10ae 100644 --- a/internal/client/tollbit/client.go +++ b/internal/client/tollbit/client.go @@ -28,6 +28,7 @@ type ( BatchGetRates(ctx context.Context, urls []string, token agent.Token) ([]BatchRateResponseV2, error) CreateContentAccessToken(ctx context.Context, req CreateContentAccessTokenRequest, token agent.Token) (CreateContentAccessTokenResponse, error) GetContent(ctx context.Context, articleURL, contentToken, userAgent string, token agent.Token) (GetContentResponse, error) + SubmitFeedback(ctx context.Context, req SubmitFeedbackRequest, token agent.Token) (SubmitFeedbackResponse, error) } client struct { @@ -139,6 +140,17 @@ type ( License BatchRateLicenseResponse `json:"license"` } + SubmitFeedbackRequest struct { + Message string `json:"message"` + Rating *int `json:"rating,omitempty"` + Category string `json:"category,omitempty"` + Metadata map[string]string `json:"metadata,omitempty"` + } + + SubmitFeedbackResponse struct { + Accepted bool `json:"accepted"` + } + requestOption func(*http.Request) ) @@ -247,6 +259,29 @@ func (c *client) GetContent(ctx context.Context, articleURL, contentToken, userA ) } +func (c *client) SubmitFeedback(ctx context.Context, req SubmitFeedbackRequest, token agent.Token) (SubmitFeedbackResponse, error) { + if err := requireAgentToken(token); err != nil { + return SubmitFeedbackResponse{}, err + } + message := strings.TrimSpace(req.Message) + if message == "" { + return SubmitFeedbackResponse{}, errors.New("feedback message is required") + } + req.Message = message + if cat := strings.TrimSpace(req.Category); cat != "" { + req.Category = cat + } else { + req.Category = "" + } + if req.Rating != nil && (*req.Rating < 1 || *req.Rating > 5) { + return SubmitFeedbackResponse{}, errors.New("rating must be between 1 and 5") + } + + u := c.resolve("/agents/v1/feedback") + var out SubmitFeedbackResponse + return out, c.doJSON(ctx, http.MethodPost, u.String(), req, &out, withBearerToken(token.RawToken)) +} + func contentResourcePath(articleURL string) (string, error) { parsed, err := url.Parse(strings.TrimSpace(articleURL)) if err != nil { diff --git a/internal/client/tollbit/client_test.go b/internal/client/tollbit/client_test.go index b8ebee8..acb2d2a 100644 --- a/internal/client/tollbit/client_test.go +++ b/internal/client/tollbit/client_test.go @@ -380,3 +380,84 @@ func TestGetContentRequiresAgentToken(t *testing.T) { t.Fatal("expected error") } } + +func TestSubmitFeedback(t *testing.T) { + token := validAgentToken(t) + rating := 4 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Fatalf("unexpected method: %s", r.Method) + } + if r.URL.Path != "/agents/v1/feedback" { + t.Fatalf("unexpected path: %s", r.URL.Path) + } + if r.Header.Get("Authorization") != "Bearer "+token.RawToken { + t.Fatalf("unexpected authorization: %q", r.Header.Get("Authorization")) + } + if r.Header.Get("User-Agent") != version.HTTPUserAgent() { + t.Fatalf("unexpected user agent: %q", r.Header.Get("User-Agent")) + } + var body SubmitFeedbackRequest + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatal(err) + } + if body.Message != "search felt off" { + t.Fatalf("unexpected message: %q", body.Message) + } + if body.Rating == nil || *body.Rating != 4 { + t.Fatalf("unexpected rating: %#v", body.Rating) + } + if body.Category != "search" { + t.Fatalf("unexpected category: %q", body.Category) + } + if body.Metadata["source"] != "cli" { + t.Fatalf("unexpected metadata: %#v", body.Metadata) + } + w.WriteHeader(http.StatusAccepted) + _ = json.NewEncoder(w).Encode(SubmitFeedbackResponse{Accepted: true}) + })) + defer srv.Close() + + c, err := NewClient(Config{BaseURL: srv.URL}) + if err != nil { + t.Fatal(err) + } + resp, err := c.SubmitFeedback(context.Background(), SubmitFeedbackRequest{ + Message: " search felt off ", + Rating: &rating, + Category: "search", + Metadata: map[string]string{"source": "cli"}, + }, token) + if err != nil { + t.Fatal(err) + } + if !resp.Accepted { + t.Fatal("expected accepted") + } +} + +func TestSubmitFeedbackRequiresMessage(t *testing.T) { + c, err := NewClient(Config{BaseURL: "https://gateway.example.com"}) + if err != nil { + t.Fatal(err) + } + _, err = c.SubmitFeedback(context.Background(), SubmitFeedbackRequest{}, validAgentToken(t)) + if err == nil || !strings.Contains(err.Error(), "feedback message is required") { + t.Fatalf("expected message required error, got %v", err) + } +} + +func TestSubmitFeedbackRejectsInvalidRating(t *testing.T) { + c, err := NewClient(Config{BaseURL: "https://gateway.example.com"}) + if err != nil { + t.Fatal(err) + } + rating := 9 + _, err = c.SubmitFeedback(context.Background(), SubmitFeedbackRequest{ + Message: "hi", + Rating: &rating, + }, validAgentToken(t)) + if err == nil || !strings.Contains(err.Error(), "rating must be between 1 and 5") { + t.Fatalf("expected rating error, got %v", err) + } +} diff --git a/skill/tollbit-cli/SKILL.md b/skill/tollbit-cli/SKILL.md index ab6da6c..0fdc1ad 100644 --- a/skill/tollbit-cli/SKILL.md +++ b/skill/tollbit-cli/SKILL.md @@ -70,12 +70,13 @@ If `auth login` or `auth complete` reports that authorization is still pending, ## For automation -- Prefer `--json` for machine-readable output on `search`, `content pricing`, `content fetch`, and `auth status`. +- Prefer `--json` for machine-readable output on `search`, `content pricing`, `content fetch`, `feedback`, and `auth status`. - **Exit codes:** `0` success · `1` runtime error · `2` usage error · `3` authorization pending (`auth login` / `auth complete` in a detached flow). `auth status --check` → `0` valid · `1` invalid/expired · `2` missing. Before paid `fetch`, prefer `auth status --check` (or `auth status --json`) so you fail fast instead of hanging on interactive consent. - **Streams:** stdout carries data only; prompts, spinners, next-step hints, and errors go to stderr. Parse stdout for success data; treat non-zero exit as failure and read stderr when diagnosing. - **Non-interactive fetch:** never call `content fetch` without `--confirm`. Pass `--rate-index N` when multiple rates exist (required with `--json` in that case). Every fetch still charges. - **Licensable results:** only **Programmatic** results can be priced and fetched; use `--programmatic-only` or skip Enterprise hits. - **Pagination:** reuse the `--next-token` / `nextToken` from the previous search response when more results exist. +- **Feedback:** use `tollbit feedback "…"` (optional `--rating`, `--category`, `--metadata key=value`) when the user wants to report CLI or product issues to Tollbit. Install this skill: `tollbit guide --install `. Compare frontmatter `version` with `tollbit version` when updating.