Skip to content

Commit 8f9402a

Browse files
authored
feat(cli): add feedback command (#201)
Adds heygen feedback (anonymous CLI_FEEDBACK PostHog event, rune-capped comment, $ip:null on all telemetry); docs steer agents away from auto-filing GitHub issues.
1 parent 438f594 commit 8f9402a

7 files changed

Lines changed: 280 additions & 2 deletions

File tree

CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ Generated commands are pure data. To add behavior on top:
8585
},
8686
```
8787

88-
**Custom commands** (not in the API spec) — add a new file in `cmd/heygen/` and register in `root.go`. See `video_download.go` for an example.
88+
**Custom commands** (not in the API spec) — add a new file in `cmd/heygen/` and register in `root.go`. See `video_download.go` for an example. Commands that don't hit the API (e.g. `feedback`, which emits an anonymous analytics event via `internal/analytics`) annotate with `skipAuth: true` so `initContext` doesn't require credentials, and call `ctx.formatter.Data(data, "", nil)` for JSON output. Register in **both** `newRootCmd` and `newRootCmdWithSpecs`.
8989

9090
**Deprecated aliases** (backward compatibility after a spec-driven rename) — register in `cmd/heygen/aliases.go`. Command names are derived from the upstream OpenAPI spec, so an upstream tag or path change can rename a command that already shipped in a stable release. An `Alias` re-registers the canonical `Spec` at its old path, hidden from help and marked deprecated, so the old invocation still resolves to the same handler while a stderr notice points to the new name:
9191

README.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Full reference and examples: **[developers.heygen.com/cli](https://developers.he
2626
- **JSON on stdout, structured errors on stderr, stable exit codes.**
2727
- **Self-describing.** `--request-schema` and `--response-schema` return JSON Schema without auth or API calls.
2828
- **Non-interactive by default.** Set `HEYGEN_API_KEY` and nothing reads a TTY.
29+
- **Tell us how it went.** After a flow works (or when you hit a bug), run `heygen feedback --rating <1-5> --comment "..."`. It sends an anonymous rating + note (no API key needed); honors the analytics opt-out. Agents should use this for bugs rather than opening GitHub issues automatically (see [Reporting bugs](#reporting-bugs)).
2930

3031
## Install
3132

@@ -187,6 +188,13 @@ Example error envelope:
187188
heygen config list # show all settings with sources
188189
```
189190

191+
## Reporting bugs
192+
193+
- **Quick signal (recommended for agents):** `heygen feedback --rating <1-5> --comment "..."`. Goes to private, anonymous analytics. Safe to run unattended.
194+
- **Tracked bug a maintainer should see:** open a [GitHub issue](https://github.com/heygen-com/heygen-cli/issues). This repo is **public**, so review what you paste first and omit anything sensitive (API keys, prompts, internal URLs, personal data).
195+
196+
**Agents:** do not open GitHub issues automatically. Use `heygen feedback` for bug signal; if something seems worth tracking, surface it to the user and let a human file the issue after reviewing it for sensitive content.
197+
190198
## Contributing
191199

192200
See [CONTRIBUTING.md](./CONTRIBUTING.md).

cmd/heygen/feedback.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"unicode/utf8"
6+
7+
"github.com/heygen-com/heygen-cli/internal/analytics"
8+
clierrors "github.com/heygen-com/heygen-cli/internal/errors"
9+
"github.com/spf13/cobra"
10+
)
11+
12+
// maxFeedbackCommentLen bounds free-text so a stray log/stack-trace paste
13+
// can't bloat the telemetry event. Generous enough for a detailed bug note;
14+
// full reports belong in a GitHub issue, not this field.
15+
const maxFeedbackCommentLen = 2000
16+
17+
type feedbackResponse struct {
18+
Rating int `json:"rating"`
19+
Comment string `json:"comment,omitempty"`
20+
Message string `json:"message"`
21+
}
22+
23+
func newFeedbackCmd(ctx *cmdContext, analyticsClient *analytics.Client) *cobra.Command {
24+
cmd := &cobra.Command{
25+
Use: "feedback",
26+
Short: "Send anonymous feedback about the CLI",
27+
Long: "Send anonymous feedback about your experience with the HeyGen CLI — a satisfaction " +
28+
"rating and an optional comment. Useful for agents to report whether a flow worked or to " +
29+
"flag a bug. Sent as an anonymous analytics event (no API key required) and honors the same " +
30+
"opt-out as usage analytics (HEYGEN_NO_ANALYTICS, or 'heygen config set analytics false').",
31+
Args: cobra.NoArgs,
32+
Annotations: map[string]string{"skipAuth": "true"},
33+
Example: " # Rate your experience and add a note\n" +
34+
" heygen feedback --rating 5 --comment \"avatar list --human is great\"\n\n" +
35+
" # Report a problem\n" +
36+
" heygen feedback --rating 2 --comment \"video download timed out on a large file\"",
37+
RunE: func(cmd *cobra.Command, args []string) error {
38+
rating, _ := cmd.Flags().GetInt("rating")
39+
comment, _ := cmd.Flags().GetString("comment")
40+
return runFeedback(ctx, analyticsClient, rating, comment)
41+
},
42+
}
43+
cmd.Flags().Int("rating", 0, "Satisfaction rating 1-5: 1 = broke / unusable, 3 = worked with friction, 5 = worked great")
44+
cmd.Flags().String("comment", "", "Optional details about your experience or the bug you hit (max 2000 characters)")
45+
_ = cmd.MarkFlagRequired("rating")
46+
return cmd
47+
}
48+
49+
func runFeedback(ctx *cmdContext, analyticsClient *analytics.Client, rating int, comment string) error {
50+
if rating < 1 || rating > 5 {
51+
return clierrors.NewUsage("rating must be between 1 and 5")
52+
}
53+
if n := utf8.RuneCountInString(comment); n > maxFeedbackCommentLen {
54+
return clierrors.NewUsage(fmt.Sprintf("comment must be %d characters or fewer (got %d); shorten it, or ask a maintainer to open a GitHub issue for a detailed report", maxFeedbackCommentLen, n))
55+
}
56+
57+
message := "Thanks for the feedback!"
58+
if !analyticsClient.Feedback(rating, comment) {
59+
message = "Analytics is disabled, so feedback was not sent. Unset HEYGEN_NO_ANALYTICS " +
60+
"(or run 'heygen config set analytics true') to enable it."
61+
}
62+
63+
data, err := marshalData(feedbackResponse{Rating: rating, Comment: comment, Message: message})
64+
if err != nil {
65+
return err
66+
}
67+
return ctx.formatter.Data(data, "", nil)
68+
}

cmd/heygen/feedback_test.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package main
2+
3+
import (
4+
"encoding/json"
5+
"strings"
6+
"testing"
7+
)
8+
9+
// assertUsageEnvelope checks stderr carries the canonical error envelope with
10+
// the usage_error code, matching the repo convention of asserting envelope
11+
// shape (not just exit code) on failures.
12+
func assertUsageEnvelope(t *testing.T, stderr string) {
13+
t.Helper()
14+
var envelope map[string]map[string]any
15+
if err := json.Unmarshal([]byte(stderr), &envelope); err != nil {
16+
t.Fatalf("stderr is not valid JSON: %v\nstderr: %s", err, stderr)
17+
}
18+
if envelope["error"]["code"] != "usage_error" {
19+
t.Errorf("error.code = %v, want %q", envelope["error"]["code"], "usage_error")
20+
}
21+
}
22+
23+
func TestFeedback_Valid(t *testing.T) {
24+
// No server and no API key: feedback is skipAuth, so it must succeed
25+
// without credentials. Analytics is disabled in runCommand, so the event
26+
// isn't sent — the command should report that rather than a false thanks.
27+
res := runCommand(t, "", "", "feedback", "--rating", "4", "--comment", "nice")
28+
if res.ExitCode != 0 {
29+
t.Fatalf("exit = %d, want 0 (stderr: %s)", res.ExitCode, res.Stderr)
30+
}
31+
32+
var got feedbackResponse
33+
if err := json.Unmarshal([]byte(res.Stdout), &got); err != nil {
34+
t.Fatalf("stdout is not valid JSON: %v\n%s", err, res.Stdout)
35+
}
36+
if got.Rating != 4 {
37+
t.Fatalf("rating = %d, want 4", got.Rating)
38+
}
39+
if got.Comment != "nice" {
40+
t.Fatalf("comment = %q, want %q", got.Comment, "nice")
41+
}
42+
if !strings.Contains(got.Message, "Analytics is disabled") {
43+
t.Fatalf("message = %q, want it to report analytics disabled", got.Message)
44+
}
45+
}
46+
47+
func TestFeedback_RatingOnly(t *testing.T) {
48+
res := runCommand(t, "", "", "feedback", "--rating", "5")
49+
if res.ExitCode != 0 {
50+
t.Fatalf("exit = %d, want 0 (stderr: %s)", res.ExitCode, res.Stderr)
51+
}
52+
if !json.Valid([]byte(res.Stdout)) {
53+
t.Fatalf("stdout is not valid JSON: %s", res.Stdout)
54+
}
55+
}
56+
57+
func TestFeedback_MissingRating(t *testing.T) {
58+
res := runCommand(t, "", "", "feedback")
59+
if res.ExitCode != 2 {
60+
t.Fatalf("exit = %d, want 2 (stdout: %s)", res.ExitCode, res.Stdout)
61+
}
62+
assertUsageEnvelope(t, res.Stderr)
63+
}
64+
65+
func TestFeedback_CommentTooLong(t *testing.T) {
66+
long := strings.Repeat("x", 2001)
67+
res := runCommand(t, "", "", "feedback", "--rating", "3", "--comment", long)
68+
if res.ExitCode != 2 {
69+
t.Fatalf("exit = %d, want 2 (stdout: %s)", res.ExitCode, res.Stdout)
70+
}
71+
assertUsageEnvelope(t, res.Stderr)
72+
}
73+
74+
func TestFeedback_CommentAtCap(t *testing.T) {
75+
atCap := strings.Repeat("x", 2000)
76+
res := runCommand(t, "", "", "feedback", "--rating", "3", "--comment", atCap)
77+
if res.ExitCode != 0 {
78+
t.Fatalf("exit = %d, want 0 (2000 chars is allowed) (stderr: %s)", res.ExitCode, res.Stderr)
79+
}
80+
}
81+
82+
func TestFeedback_CommentCapCountsRunesNotBytes(t *testing.T) {
83+
// 2000 multibyte runes is 4000 bytes; it must pass because the cap is on
84+
// characters, not bytes.
85+
multibyte := strings.Repeat("é", 2000)
86+
res := runCommand(t, "", "", "feedback", "--rating", "3", "--comment", multibyte)
87+
if res.ExitCode != 0 {
88+
t.Fatalf("exit = %d, want 0 (2000 runes is allowed) (stderr: %s)", res.ExitCode, res.Stderr)
89+
}
90+
}
91+
92+
func TestFeedback_RatingOutOfRange(t *testing.T) {
93+
for _, r := range []string{"0", "6", "9"} {
94+
res := runCommand(t, "", "", "feedback", "--rating", r)
95+
if res.ExitCode != 2 {
96+
t.Fatalf("rating %s: exit = %d, want 2 (stdout: %s)", r, res.ExitCode, res.Stdout)
97+
}
98+
assertUsageEnvelope(t, res.Stderr)
99+
if !strings.Contains(res.Stderr, "rating must be between 1 and 5") {
100+
t.Errorf("rating %s: stderr missing range message: %s", r, res.Stderr)
101+
}
102+
}
103+
}

cmd/heygen/root.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ func newRootCmd(version string, formatter output.Formatter, analyticsClient *ana
4747
root.AddCommand(newAuthCmd(ctx))
4848
root.AddCommand(newConfigCmd(ctx))
4949
root.AddCommand(newUpdateCmd(ctx))
50+
root.AddCommand(newFeedbackCmd(ctx, analyticsClient))
5051
registerGroups(root, ctx, gen.Groups)
5152
attachCustomCommands(root, ctx)
5253
attachDeprecatedAliases(root, ctx)
@@ -126,6 +127,7 @@ func newRootCmdWithSpecs(version string, formatter output.Formatter, analyticsCl
126127
root.AddCommand(newAuthCmd(ctx))
127128
root.AddCommand(newConfigCmd(ctx))
128129
root.AddCommand(newUpdateCmd(ctx))
130+
root.AddCommand(newFeedbackCmd(ctx, analyticsClient))
129131
registerGroups(root, ctx, groups)
130132
attachCustomCommands(root, ctx)
131133
attachDeprecatedAliases(root, ctx)

internal/analytics/analytics.go

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,27 @@ func (c *Client) CommandRunComplete(command string, exitCode int, duration time.
8585
})
8686
}
8787

88+
// Feedback records a satisfaction rating (1-5) with an optional free-text
89+
// comment as a CLI_FEEDBACK event. Returns false when analytics is disabled
90+
// (opt-out), so the caller can tell the user nothing was sent.
91+
func (c *Client) Feedback(rating int, comment string) bool {
92+
if !c.enabled || c.ph == nil {
93+
return false
94+
}
95+
props := c.baseProperties("feedback").Set("rating", rating)
96+
// Omit an empty comment rather than sending "" — an empty value and an
97+
// absent one are different cohorts in analysis (cf. client_origin).
98+
if comment != "" {
99+
props = props.Set("comment", comment)
100+
}
101+
_ = c.ph.Enqueue(posthog.Capture{
102+
DistinctId: c.distinctID,
103+
Event: "CLI_FEEDBACK",
104+
Properties: props,
105+
})
106+
return true
107+
}
108+
88109
// baseProperties is the per-event property bundle every CLI event carries.
89110
// Kept in one place so client_origin / cli_version / os / arch can't drift
90111
// between COMMAND_RUN and COMMAND_RUN_COMPLETE — funnel queries break when
@@ -94,7 +115,11 @@ func (c *Client) baseProperties(command string) posthog.Properties {
94115
Set("command", command).
95116
Set("cli_version", c.version).
96117
Set("os", runtime.GOOS).
97-
Set("arch", runtime.GOARCH)
118+
Set("arch", runtime.GOARCH).
119+
// Send $ip: null so PostHog neither stores the caller's IP nor geolocates
120+
// it. The CLI is opt-in anonymous telemetry; IP would undermine that. PostHog
121+
// otherwise derives IP from the ingest request, so this must be set explicitly.
122+
Set("$ip", nil)
98123
if c.clientOrigin != "" {
99124
props = props.Set("client_origin", c.clientOrigin)
100125
}

internal/analytics/analytics_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,78 @@ func TestCommandRun_OmitsClientOriginWhenEmpty(t *testing.T) {
131131
}
132132
}
133133

134+
func TestFeedback_Properties(t *testing.T) {
135+
stub := &stubCaptureClient{}
136+
client := newWithCapture("v1.2.3", stub)
137+
client.distinctID = "anon-id"
138+
139+
if !client.Feedback(5, "love it") {
140+
t.Fatal("Feedback returned false for an enabled client")
141+
}
142+
if len(stub.messages) != 1 {
143+
t.Fatalf("messages = %d, want 1", len(stub.messages))
144+
}
145+
146+
msg, ok := stub.messages[0].(posthog.Capture)
147+
if !ok {
148+
t.Fatalf("message type = %T, want posthog.Capture", stub.messages[0])
149+
}
150+
if msg.Event != "CLI_FEEDBACK" {
151+
t.Fatalf("Event = %q, want %q", msg.Event, "CLI_FEEDBACK")
152+
}
153+
if msg.DistinctId != "anon-id" {
154+
t.Fatalf("DistinctId = %q, want %q", msg.DistinctId, "anon-id")
155+
}
156+
if got := msg.Properties["rating"]; got != 5 {
157+
t.Fatalf("rating = %v, want 5", got)
158+
}
159+
if got := msg.Properties["comment"]; got != "love it" {
160+
t.Fatalf("comment = %v, want %q", got, "love it")
161+
}
162+
if got := msg.Properties["cli_version"]; got != "v1.2.3" {
163+
t.Fatalf("cli_version = %v, want %q", got, "v1.2.3")
164+
}
165+
}
166+
167+
// An empty comment must not land as comment:"" — the absent and empty cohorts
168+
// are distinct in analysis (same reasoning as client_origin).
169+
func TestFeedback_OmitsCommentWhenEmpty(t *testing.T) {
170+
stub := &stubCaptureClient{}
171+
client := newWithCapture("v1.2.3", stub)
172+
173+
client.Feedback(3, "")
174+
175+
msg := stub.messages[0].(posthog.Capture)
176+
if _, present := msg.Properties["comment"]; present {
177+
t.Fatalf("comment present (%v) despite empty input", msg.Properties["comment"])
178+
}
179+
}
180+
181+
// Anonymity guarantee: every event must carry $ip=null so PostHog neither
182+
// stores nor geolocates the caller's IP.
183+
func TestBaseProperties_SuppressesIP(t *testing.T) {
184+
stub := &stubCaptureClient{}
185+
client := newWithCapture("v1.2.3", stub)
186+
187+
client.CommandRun("heygen video list")
188+
client.Feedback(5, "ok")
189+
190+
for i, m := range stub.messages {
191+
props := m.(posthog.Capture).Properties
192+
ip, present := props["$ip"]
193+
if !present || ip != nil {
194+
t.Fatalf("message %d: $ip = %v (present=%v), want present and nil", i, ip, present)
195+
}
196+
}
197+
}
198+
199+
func TestFeedback_DisabledReturnsFalse(t *testing.T) {
200+
client := New("test", false)
201+
if client.Feedback(5, "x") {
202+
t.Fatal("Feedback returned true for a disabled client")
203+
}
204+
}
205+
134206
func TestClose_DisabledNoop(t *testing.T) {
135207
client := New("test", false)
136208
client.Close()

0 commit comments

Comments
 (0)