Skip to content

Commit 6d8bc96

Browse files
somanshreddyclaude
andcommitted
cli: add reserved cli_ prefix + code registry with enforcement test
Namespace the CLI-originated response-parse code as cli_response_parse_error, add a central registry of all CLI-minted codes, and a test that scans source and fails on any unregistered or bare-but-not-allowlisted CLI code, forcing the cli_ prefix on future codes so they can never collide with an API code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1f5d652 commit 6d8bc96

6 files changed

Lines changed: 202 additions & 7 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ Every command supports `--help`.
164164
| Aspect | Behavior |
165165
|--------|----------|
166166
| **stdout** | Always JSON. Even `video download` — binary writes to disk; stdout emits `{"asset", "message", "path"}` so you can chain on `.path`. |
167-
| **stderr** | Structured envelope on error: `{"error": {"code", "message", "hint", "param", "doc_url", "request_id"}}`. `code`/`message` are always present; `hint`/`param`/`doc_url`/`request_id` appear when applicable (`param`/`doc_url` are surfaced from the API for validation and documented errors). Stable `code` values for programmatic branching. |
167+
| **stderr** | Structured envelope on error: `{"error": {"code", "message", "hint", "param", "doc_url", "request_id"}}`. `code`/`message` are always present; `hint`/`param`/`doc_url`/`request_id` appear when applicable (`param`/`doc_url` are surfaced from the API for validation and documented errors). Stable `code` values for programmatic branching. A code prefixed `cli_` is originated by the CLI itself (client/transport/local conditions, e.g. `cli_download_url_expired`); a bare code carries API semantics. The `cli_` prefix is reserved for the CLI, so the two never collide. |
168168
| **Exit codes** | `0` ok · `1` API or network · `2` usage · `3` auth / not permitted · `4` timeout under `--wait` (stdout contains partial resource for resume) |
169169
| **Request bodies** | Flags for simple inputs; `-d` for nested JSON (inline, file path, or `-` for stdin). Flags override matching fields. |
170170
| **Async jobs** | `--wait` blocks with exponential backoff; `--timeout` sets max (default 20m). 429s and 5xx retry automatically. |

internal/analytics/analytics_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ func TestCommandRunComplete_SourceAndHTTPStatus(t *testing.T) {
242242
t.Run("cli source omits http_status when 0", func(t *testing.T) {
243243
stub := &stubCaptureClient{}
244244
client := newWithCapture("v1.2.3", stub)
245-
client.CommandRunComplete("heygen video create", 1, time.Second, "file_io_error", "cli", 0)
245+
client.CommandRunComplete("heygen video create", 1, time.Second, "cli_file_io_error", "cli", 0)
246246
msg := stub.messages[0].(posthog.Capture)
247247
if got := msg.Properties["source"]; got != "cli" {
248248
t.Fatalf("source = %v, want cli", got)

internal/client/executor.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (c *Client) ExecuteAndPoll(
8282
resourceID, err := extractJSONPath(createResp, spec.PollConfig.IDField)
8383
if err != nil {
8484
return nil, &clierrors.CLIError{
85-
Code: "response_parse_error",
85+
Code: "cli_response_parse_error",
8686
Message: fmt.Sprintf(
8787
"failed to extract resource ID from %q: %v", spec.PollConfig.IDField, err),
8888
Hint: "The create response did not have the expected shape. This command may require manual polling for batch responses.",
@@ -135,7 +135,7 @@ func (c *Client) ExecuteAndPoll(
135135
status, err := extractJSONPath(statusResp, spec.PollConfig.StatusField)
136136
if err != nil {
137137
return nil, &clierrors.CLIError{
138-
Code: "response_parse_error",
138+
Code: "cli_response_parse_error",
139139
Message: fmt.Sprintf(
140140
"failed to extract status from %q: %v", spec.PollConfig.StatusField, err),
141141
Hint: "The status response did not have the expected shape. Retry; if it persists, report it.",
@@ -207,6 +207,10 @@ func (c *Client) executeWithContext(ctx context.Context, spec *command.Spec, inv
207207
ExitCode: clierrors.ExitAuth,
208208
}
209209
}
210+
// network_error is a grandfathered bare code (not cli_-prefixed): it is
211+
// shared with the download-command transport path (cmd/heygen/video_download.go)
212+
// and predates the cli_ convention. See the grandfathered set in
213+
// internal/errors/codes.go; keep both emission sites bare and in sync.
210214
return nil, &clierrors.CLIError{
211215
Code: "network_error",
212216
Message: fmt.Sprintf("request failed: %v", err),
@@ -407,7 +411,7 @@ func extractJSONPath(raw json.RawMessage, path string) (string, error) {
407411
var current any
408412
if err := json.Unmarshal(raw, &current); err != nil {
409413
return "", &clierrors.CLIError{
410-
Code: "response_parse_error",
414+
Code: "cli_response_parse_error",
411415
Message: fmt.Sprintf("failed to parse JSON response: %v", err),
412416
Hint: "The API response could not be parsed. Retry; if it persists, report it (possible CLI/API mismatch).",
413417
ExitCode: clierrors.ExitGeneral,

internal/client/executor_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,8 +1021,8 @@ func TestRequestIDFromHeaders(t *testing.T) {
10211021
func TestExtractJSONPath_ParseError(t *testing.T) {
10221022
_, err := extractJSONPath([]byte("{not valid json"), "data.id")
10231023
var cliErr *clierrors.CLIError
1024-
if !errors.As(err, &cliErr) || cliErr.Code != "response_parse_error" {
1025-
t.Fatalf("err = %v, want a response_parse_error CLIError", err)
1024+
if !errors.As(err, &cliErr) || cliErr.Code != "cli_response_parse_error" {
1025+
t.Fatalf("err = %v, want a cli_response_parse_error CLIError", err)
10261026
}
10271027
}
10281028

internal/errors/codes.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package errors
2+
3+
// CLI error-code namespace governance.
4+
//
5+
// A code is "CLI-originated" when the CLI synthesizes it itself, as opposed to
6+
// relaying a code from the API response envelope. Every CLI-originated code must
7+
// be registered in exactly one of the three sets below. New CLI-originated codes
8+
// MUST carry the reserved cli_ prefix (cliPrefixedCodes); the two bare sets are
9+
// CLOSED allowlists (grandfathered legacy + API-semantic) that must not grow.
10+
//
11+
// The cli_ prefix is reserved so a CLI code can never collide with a future API
12+
// code, provided the API never mints a cli_-prefixed code (enforced API-side).
13+
// codes_test.go scans the source and fails on any CLI-minted code that is either
14+
// unregistered or bare-but-not-allowlisted, which forces the prefix on new codes.
15+
16+
// CLICodePrefix is reserved for CLI-originated error codes. The API must never
17+
// emit a code beginning with this prefix.
18+
const CLICodePrefix = "cli_"
19+
20+
// cliPrefixedCodes are CLI-originated codes carrying the reserved prefix.
21+
var cliPrefixedCodes = []string{
22+
"cli_response_encode_error",
23+
"cli_response_parse_error",
24+
"cli_download_url_expired",
25+
"cli_download_failed",
26+
"cli_download_interrupted",
27+
"cli_file_io_error",
28+
}
29+
30+
// grandfatheredBareCodes are CLI-originated codes that shipped bare in a stable
31+
// release before the cli_ convention. FROZEN: do not add. Their contract is the
32+
// exit code (buckets) or they are CLI-only names the API has no reason to mint.
33+
var grandfatheredBareCodes = []string{
34+
"error", "usage_error", "auth_error", "timeout", "canceled",
35+
"network_error", "confirmation_required", "file_exists",
36+
"batch_not_supported", "wrong_install_method",
37+
"video_failed", "video_not_ready",
38+
}
39+
40+
// bareAPISemanticCodes are bare codes that carry API semantics: CLI mirrors of
41+
// API codes (same name, same meaning, so a same-name overlap is correct, not a
42+
// clash) and CLI fallback labels for API faults. Not reverse-collision risk;
43+
// unclassified_* is an accepted risk (the API will not mint an "unclassified"
44+
// code). asset_not_available is dual-use: an API code the CLI also mints.
45+
var bareAPISemanticCodes = []string{
46+
"not_found", "insufficient_credit", "forbidden", "unauthorized",
47+
"conflict", "rate_limit_exceeded", "validation_error", "payload_too_large",
48+
"asset_not_available",
49+
"unclassified_server_error", "unclassified_client_error",
50+
}

internal/errors/codes_test.go

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package errors
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"regexp"
7+
"runtime"
8+
"strings"
9+
"testing"
10+
)
11+
12+
// codeLiteral matches a CLI-minted error-code string literal: either a `Code:`
13+
// struct field or a `code =` assignment (the two forms the CLI uses to
14+
// synthesize a code). API codes relayed from a response come from a variable
15+
// (apiErr.Code), not a literal, so they do not match.
16+
var codeLiteral = regexp.MustCompile(`(?:\bCode:\s*|\bcode\s*=\s*)"([a-z][a-z0-9_]*)"`)
17+
18+
func registrySet(t *testing.T) map[string]string {
19+
t.Helper()
20+
m := map[string]string{}
21+
add := func(codes []string, bucket string) {
22+
for _, c := range codes {
23+
if prev, dup := m[c]; dup {
24+
t.Fatalf("code %q registered in both %q and %q", c, prev, bucket)
25+
}
26+
m[c] = bucket
27+
}
28+
}
29+
add(cliPrefixedCodes, "cliPrefixed")
30+
add(grandfatheredBareCodes, "grandfatheredBare")
31+
add(bareAPISemanticCodes, "bareAPISemantic")
32+
return m
33+
}
34+
35+
// TestCLICodePartition enforces the namespace invariants on the registry.
36+
func TestCLICodePartition(t *testing.T) {
37+
for _, c := range cliPrefixedCodes {
38+
if !strings.HasPrefix(c, CLICodePrefix) {
39+
t.Errorf("cliPrefixedCodes: %q must start with %q", c, CLICodePrefix)
40+
}
41+
}
42+
for _, c := range append(append([]string{}, grandfatheredBareCodes...), bareAPISemanticCodes...) {
43+
if strings.HasPrefix(c, CLICodePrefix) {
44+
t.Errorf("bare code %q must not start with %q (only cliPrefixedCodes may)", c, CLICodePrefix)
45+
}
46+
}
47+
}
48+
49+
// TestBareAllowlistsAreFrozen pins the exact contents of the two bare sets.
50+
// The reserved-prefix guarantee relies on these being CLOSED: without this,
51+
// a new bare CLI code could bypass the cli_ rule by simply being appended to
52+
// grandfatheredBareCodes. Changing either set now requires editing this
53+
// expected list too, which surfaces the change in review.
54+
func TestBareAllowlistsAreFrozen(t *testing.T) {
55+
wantGrandfathered := []string{
56+
"auth_error", "batch_not_supported", "canceled", "confirmation_required",
57+
"error", "file_exists", "network_error", "timeout", "usage_error",
58+
"video_failed", "video_not_ready", "wrong_install_method",
59+
}
60+
wantAPISemantic := []string{
61+
"asset_not_available", "conflict", "forbidden", "insufficient_credit",
62+
"not_found", "payload_too_large", "rate_limit_exceeded", "unauthorized",
63+
"unclassified_client_error", "unclassified_server_error", "validation_error",
64+
}
65+
assertSetEqual(t, "grandfatheredBareCodes", grandfatheredBareCodes, wantGrandfathered)
66+
assertSetEqual(t, "bareAPISemanticCodes", bareAPISemanticCodes, wantAPISemantic)
67+
}
68+
69+
func assertSetEqual(t *testing.T, name string, got, want []string) {
70+
t.Helper()
71+
gm := map[string]bool{}
72+
for _, c := range got {
73+
gm[c] = true
74+
}
75+
wm := map[string]bool{}
76+
for _, c := range want {
77+
wm[c] = true
78+
}
79+
for c := range gm {
80+
if !wm[c] {
81+
t.Errorf("%s: unexpected code %q — if this is a new bare CLI code, it likely must be cli_-prefixed instead; only add to the frozen set with explicit justification", name, c)
82+
}
83+
}
84+
for c := range wm {
85+
if !gm[c] {
86+
t.Errorf("%s: missing expected code %q", name, c)
87+
}
88+
}
89+
}
90+
91+
// TestNoUnregisteredCLICodes scans production source for CLI-minted code
92+
// literals and fails on any that is not registered. A new CLI code therefore
93+
// must be added to the registry, and (unless grandfathered/API-semantic) must
94+
// carry the cli_ prefix, which is what keeps future codes collision-proof.
95+
func TestNoUnregisteredCLICodes(t *testing.T) {
96+
reg := registrySet(t)
97+
root := repoRoot(t)
98+
99+
// Codes the CLI never originates: relayed from tests as API-response bodies,
100+
// or test-only fixtures. These are not CLI-minted, so exclude them.
101+
ignore := map[string]bool{}
102+
103+
for _, dir := range []string{"cmd", "internal"} {
104+
err := filepath.WalkDir(filepath.Join(root, dir), func(path string, d os.DirEntry, err error) error {
105+
if err != nil {
106+
return err
107+
}
108+
if d.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
109+
return nil
110+
}
111+
src, err := os.ReadFile(path)
112+
if err != nil {
113+
return err
114+
}
115+
for _, m := range codeLiteral.FindAllStringSubmatch(string(src), -1) {
116+
code := m[1]
117+
if ignore[code] {
118+
continue
119+
}
120+
if _, ok := reg[code]; !ok {
121+
rel, _ := filepath.Rel(root, path)
122+
t.Errorf("unregistered CLI-minted code %q in %s: prefix it with %q and add it to cliPrefixedCodes in codes.go (or, if it mirrors an API code / is grandfathered, add it to the matching bare set)", code, rel, CLICodePrefix)
123+
}
124+
}
125+
return nil
126+
})
127+
if err != nil {
128+
t.Fatalf("walk %s: %v", dir, err)
129+
}
130+
}
131+
}
132+
133+
func repoRoot(t *testing.T) string {
134+
t.Helper()
135+
_, file, _, ok := runtime.Caller(0)
136+
if !ok {
137+
t.Fatal("runtime.Caller failed")
138+
}
139+
// this file lives at <root>/internal/errors/codes_test.go
140+
return filepath.Clean(filepath.Join(filepath.Dir(file), "..", ".."))
141+
}

0 commit comments

Comments
 (0)