docs(release): add a command-surface regression check to the release gate (PRINFRA-499) (#284)

The command surface is generated from an upstream OpenAPI spec this repo does
not control, so a resync can remove a command, rename a flag, flip one to
required, or reroute what the CLI sends, with no commit in this repo saying so.
The resync lands as a single "codegen: resync gen/" line and /changelog-cli
collapses it into Internal, so the commit log and the changelog — the two things
a releaser reads — are exactly where such a break is invisible.

Adds a required pre-release step that reduces gen/ to a normalized manifest of
contract-bearing fields at two refs and diffs them. The grep is an allowlist
rather than a denylist: a field omitted from a denylist is invisible forever,
and the first draft of this check proved the point by filtering out Source,
BodyEncoding and JSONName, each of which reroutes the request. The allowlist now
covers every field gen/ emits except the four that are pure prose. Request and
response schemas are collapsed to a presence marker instead of dropped, since
their contents churn constantly but their presence decides whether
--request-schema and --response-schema exist.

Verified against a worktree carrying six simultaneous breaks — a newly added
already-required flag, Source query->body, a deleted RequestSchema, BodyEncoding
json->multipart, a dropped Destructive, and a JSONName rename: the manifest diff
surfaces all six. The v0.6.0..main range correctly produces empty output.

A shape diff cannot see a flag that keeps its signature and stops doing
anything, which is what just happened to --enable-caption on video-translate
and lipsync, so a second pass greps new help text for deprecation language.

Also corrects the bump rule, which listed codegen resyncs as patch-worthy
without qualification and would have mislabeled a breaking resync as a patch.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: somanshreddy

RELEASE.md

@@ -33,23 +33,33 @@ gh workflow run dev-release.yml
 
 ### Pre-release checklist
 
+Every step compares against `origin/main` and the last stable tag, so start from a fetched checkout:
+
+```bash
+git fetch --tags origin
+LAST_STABLE=$(git tag --list 'v*' --sort=-v:refname | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | head -n 1)
+```
+
+Keep `$LAST_STABLE` set for the rest of the checklist; steps 4 and 6 reuse it. It filters to release tags, so a run of `-dev.*` prereleases can't shadow the stable one.
+
 1. **Review commits since last stable.** Check what's new and confirm nothing is half-finished:
    ```bash
-   git log $(gh release view --json tagName -q .tagName)..origin/main --oneline
+   git log "$LAST_STABLE"..origin/main --oneline
    ```
 2. **Check open PRs.** Decide if any should merge first (e.g. pending codegen resyncs, small fixes):
    ```bash
    gh pr list --state open
    ```
 3. **Confirm CI is green on main.** All checks should pass on the latest commit.
-4. **Run E2E smoke test.** With `HEYGEN_API_KEY` set, run `/e2e-cli-test` in Claude Code from the repo root. Confirm all phases pass (no FAIL). WARN on Phase 3 means the account lacks data for some get/detail commands and should be investigated. This builds the binary and exercises it against the live API (costs a small number of credits).
-5. **Pick the version number.** Check the last stable tag and bump according to the rules below:
+4. **Diff the generated command surface for regressions.** See [Checking for Regressions](#checking-for-regressions) below. Required on every stable release, not just ones that look risky — a resync that breaks the CLI looks identical in `git log` to one that doesn't. It covers `gen/` only; hand-written commands in `cmd/heygen/` are reviewed the normal way, through their PRs.
+5. **Run E2E smoke test.** With `HEYGEN_API_KEY` set, run `/e2e-cli-test` in Claude Code from the repo root. Confirm all phases pass (no FAIL). WARN on Phase 3 means the account lacks data for some get/detail commands and should be investigated. This builds the binary and exercises it against the live API (costs a small number of credits).
+6. **Pick the version number.** Check the last stable tag and bump according to the rules below:
    - Patch (`v0.0.x`) for bug fixes, UX polish, codegen resyncs, and additive schema changes.
-   - Minor (`v0.x.0`) for new command groups or significant new capabilities.
+   - Minor (`v0.x.0`) for new command groups, significant new capabilities, or **any breaking surface change found in step 4** — a resync is only a patch when it is purely additive.
    ```bash
-   gh release list --limit 3
+   echo "$LAST_STABLE"
    ```
-6. **Generate changelog.** Run `/changelog-cli v0.x.y` in Claude Code. Review the output and save it for the release notes.
+7. **Generate changelog.** Run `/changelog-cli v0.x.y` in Claude Code. Review the output and save it for the release notes. The skill reads `git log`, so it cannot see the step 4 findings — add those to the release notes yourself, at the top, under **Breaking changes** if any was breaking and **Deprecated** otherwise.
 
 ### Trigger the release
 
@@ -69,16 +79,48 @@ installer, checksums, and platform archives to S3 for CDN-backed installs.
 CDN propagation takes up to 1 minute for the version pointer and 5 minutes
 for the install script.
 
-5. **Verify the release was published:**
+1. **Verify the release was published:**
    ```bash
    gh release view v0.0.5
    ```
-6. **Verify the install script picks up the new version** (after CDN propagation):
+2. **Verify the install script picks up the new version** (after CDN propagation):
    ```bash
    curl -fsSL https://static.heygen.ai/cli/install.sh | bash
    heygen --version
    ```
 
+## Checking for Regressions
+
+`gen/` is generated from HeyGen's OpenAPI spec, which lives upstream. A resync lands as one `codegen: resync gen/ from EF <sha>` commit and `/changelog-cli` files it under Internal — so the commit log and the changelog, the two things a releaser reads, are exactly where a breaking change is invisible. Diff the generated surface instead. This covers `gen/` only; hand-written commands in `cmd/heygen/` and the hidden-endpoint list in `internal/command/hidden.go` are reviewed through their own PRs.
+
+```bash
+scripts/release-surface.sh diff "$LAST_STABLE" origin/main
+```
+
+The script reduces `gen/` at both refs to the fields that decide what a user can type, then diffs the two. It filters with an allowlist, since a field left out is invisible forever; `codegen/surface_allowlist_test.go` fails the build if codegen gains a field the allowlist misses, so the list cannot rot. Request and response schemas are compared by presence rather than content: their bodies churn on every resync, but whether a command *has* one decides whether `--request-schema` and `--response-schema` exist.
+
+Empty output means the surface is unchanged. The script exits non-zero and says so if its own reduction matched nothing, because "no changes" and "the check is broken" otherwise look identical. Read the `<` lines (the old side) first — a removal is a break, an addition usually isn't.
+
+| A `<` line showing... | Effect | Action |
+|---|---|---|
+| a command or flag `Name` gone | `unknown command` / `unknown flag`, exit 2 | For a command, re-register the old path in `cmd/heygen/aliases.go`. There is no flag equivalent, so call it out. |
+| a stricter input: `Required` false→true, a dropped `Enum` value, narrowed `Min`/`Max`, changed `Type` | previously valid invocations now rejected before the request is sent | Breaking. Name the command, the flag, and the old vs new constraint. |
+| `Args` losing an entry, or a changed `Param` | positional arity changed, or the same argument now fills a different URL slot | Breaking. Show the old and new call shape. |
+| a changed `Source`, `JSONName`, `Default`, or `SendDefaultWhenOmitted` | same input, different request — routing, wire key, or whether a value is sent at all | Confirm it is intended; none of these change the help text, so nothing else will surface them. |
+| `BodyEncoding` leaving `json` | `-d/--data` is no longer registered — the builder adds it only when `BodyEncoding` is exactly `json` | Breaking for anyone passing a raw body. |
+| a `RequestSchema`/`ResponseSchema` `<present>` line gone | `--request-schema` / `--response-schema` no longer exist there | Breaking for agents that introspect before calling. |
+| `Destructive`, `Endpoint`, or `Method` changing | `--force` and the confirmation prompt appear or disappear; or the command now calls something else | Rarely intended. Confirm before releasing. |
+
+Additions are usually safe, with two exceptions that show up only as `>` lines: a new flag that is already `Required: true`, and a new `Args` entry. Both make every prior invocation of that command exit 2. (`Deprecated: true` appearing is not a break — the flag still works and still sends its value — but it is worth a release note.)
+
+Finally, a flag can stop doing anything without changing shape at all, which the diff above cannot see because it excludes help text:
+
+```bash
+scripts/release-surface.sh deprecated "$LAST_STABLE" origin/main
+```
+
+Each line names a command and the flag that went quiet, e.g. `video-translate create --enable-caption`. It matches loosely on purpose; a false positive is obvious once you can see which flag it named. A real hit belongs in the release notes, because a user whose script sets that flag gets no error and no warning — just different output than they asked for.
+
 ## Version Scheme
 
 All versions use semver with a `v` prefix. The `v` prefix is required everywhere: git tags, `--version` output, `heygen update --version` input, JSON responses, and install script flags.

codegen/surface_allowlist_test.go

@@ -0,0 +1,108 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+)
+
+// The release-time surface check (scripts/release-surface.sh, documented in
+// RELEASE.md) reduces gen/ to the fields that decide what a user can type. It
+// filters with an allowlist, so a field the template emits but the allowlist
+// omits is invisible to the check forever — a regression in it would pass a
+// release gate that reports "clean".
+//
+// This shipped broken once: SendDefaultWhenOmitted, which decides whether a
+// value is sent when the user omits the flag, was emitted by the template and
+// missing from the allowlist. It escaped review because the inventory was taken
+// over gen/ rather than the template, and no endpoint currently triggers its
+// emit condition — so it was absent from the output while being one spec change
+// away from appearing. Hence this test reads the template, not gen/.
+//
+// Excluded on purpose:
+//   - prose fields, which churn on nearly every resync and are not contract
+//   - container lines, whose entries are matched individually by the allowlist
+var (
+	proseFields     = map[string]bool{"Description": true, "Summary": true, "Help": true, "Examples": true}
+	containerFields = map[string]bool{"Flags": true, "Args": true}
+
+	templateFieldRe = regexp.MustCompile(`(?m)^\s*([A-Za-z][A-Za-z0-9]*):`)
+	allowlistRe     = regexp.MustCompile(`(?m)^SURFACE_FIELDS='(.*)'$`)
+)
+
+func TestSurfaceAllowlistCoversEveryTemplateField(t *testing.T) {
+	tmpl, err := os.ReadFile(filepath.Join("templates", "command.go.tmpl"))
+	if err != nil {
+		t.Fatalf("read template: %v", err)
+	}
+	script, err := os.ReadFile(filepath.Join("..", "scripts", "release-surface.sh"))
+	if err != nil {
+		t.Fatalf("read release-surface.sh: %v", err)
+	}
+
+	m := allowlistRe.FindSubmatch(script)
+	if m == nil {
+		t.Fatal("no SURFACE_FIELDS assignment found in scripts/release-surface.sh — " +
+			"if it was renamed or reformatted, update this test, because without it the allowlist can rot silently")
+	}
+	allowed := make(map[string]bool)
+	for _, alt := range strings.Split(string(m[1]), "|") {
+		// Alternatives look like `Group:` or `\{(Name` — take the leading identifier.
+		if name := regexp.MustCompile(`[A-Za-z][A-Za-z0-9]*`).FindString(alt); name != "" {
+			allowed[name] = true
+		}
+	}
+
+	var missing []string
+	for _, f := range templateFieldRe.FindAllSubmatch(tmpl, -1) {
+		name := string(f[1])
+		if allowed[name] || proseFields[name] || containerFields[name] {
+			continue
+		}
+		missing = append(missing, name)
+	}
+
+	if len(missing) > 0 {
+		t.Errorf("codegen/templates/command.go.tmpl emits %v, which SURFACE_FIELDS in "+
+			"scripts/release-surface.sh does not match. A change to that field would be invisible to the "+
+			"release regression check. Add it to the allowlist, or to proseFields/containerFields here if it "+
+			"genuinely is not part of the command surface.", missing)
+	}
+}
+
+// The allowlist is only meaningful if it actually selects the lines it claims
+// to. Guards against a well-formed regex that matches nothing — the failure
+// mode the script's own runtime check exists for, caught here at build time.
+func TestSurfaceAllowlistMatchesRealGeneratedLines(t *testing.T) {
+	script, err := os.ReadFile(filepath.Join("..", "scripts", "release-surface.sh"))
+	if err != nil {
+		t.Fatalf("read release-surface.sh: %v", err)
+	}
+	m := allowlistRe.FindSubmatch(script)
+	if m == nil {
+		t.Fatal("no SURFACE_FIELDS assignment found")
+	}
+	// Go's regexp is RE2; the pattern is a POSIX ERE alternation, which is a
+	// compatible subset for the constructs used here.
+	re, err := regexp.Compile(`^\s*(` + string(m[1]) + `)`)
+	if err != nil {
+		t.Fatalf("SURFACE_FIELDS is not a valid pattern: %v", err)
+	}
+
+	sample, err := os.ReadFile(filepath.Join("..", "gen", "video.go"))
+	if err != nil {
+		t.Fatalf("read gen/video.go: %v", err)
+	}
+	var matched int
+	for _, line := range strings.Split(string(sample), "\n") {
+		if re.MatchString(line) {
+			matched++
+		}
+	}
+	if matched == 0 {
+		t.Error("SURFACE_FIELDS matched no line in gen/video.go — the release check would report every " +
+			"release as clean")
+	}
+}

scripts/release-surface.sh

@@ -0,0 +1,84 @@
+#!/usr/bin/env bash
+# Release-time regression checks over the generated command surface.
+#
+# See RELEASE.md "Checking for Regressions" for how to read the output.
+#
+#   release-surface.sh diff       <old-ref> <new-ref>   what changed in the surface
+#   release-surface.sh deprecated <old-ref> <new-ref>   flags that newly went quiet
+#
+# Both checks fail toward printing nothing, and nothing looks identical to a
+# clean bill, so both assert their own input is sane before reporting.
+set -uo pipefail
+
+# Every field in gen/ that decides what a user can type.
+#
+# This is an ALLOWLIST, and that direction is load-bearing: a field left out is
+# invisible to the check forever. It is not maintained by hand alone —
+# codegen/surface_allowlist_test.go fails when the template gains a field that
+# is missing here, so the list cannot silently rot.
+#
+# Excluded on purpose: the four pure-prose fields (Description, Summary, Help,
+# Examples), which churn on nearly every resync and are not part of the
+# contract, and the two container lines (Flags, Args), whose entries are matched
+# individually instead.
+SURFACE_FIELDS='var [A-Za-z0-9_]+ = &command\.Spec\{|Group:|Name:|Endpoint:|Method:|BodyEncoding:|Paginated:|Destructive:|Deprecated:|RequestSchema:|ResponseSchema:|Required:|Enum:|Min:|Max:|Type:|Default:|Source:|JSONName:|SendDefaultWhenOmitted:|\{(Name|Param):'
+
+die() { echo "release-surface: $*" >&2; exit 1; }
+
+gen_at() {
+  # -r so a future move to subdirectories under gen/ is not silently dropped.
+  git ls-tree -r --name-only "$1" gen/ | while read -r f; do git show "$1:$f"; done
+}
+
+# Reduce gen/ to just the contract-bearing lines.
+#
+# Request and response schemas are collapsed to a marker rather than dropped:
+# their contents churn on every resync, but whether a command *has* one decides
+# whether --request-schema and --response-schema exist.
+surface() {
+  gen_at "$1" \
+    | sed -E 's/^([[:space:]]*(RequestSchema|ResponseSchema)):.*/\1: <present>/' \
+    | grep -E "^[[:space:]]*($SURFACE_FIELDS)"
+}
+
+# Flags whose help text says they stopped doing anything. Invisible to the
+# surface diff, which excludes help text by design, so this reads it directly
+# and reports the owning command to make the finding writeable as a release note.
+deprecated_flags() {
+  gen_at "$1" \
+    | awk -F'"' '
+        /^\tGroup:/     { grp=$2 }
+        /^\tName:/      { cmd=$2 }
+        /^\t\t\tName:/  { flag=$2 }
+        /^\t\t\tHelp:.*([Dd]eprecat|no longer|ignored)/ { print grp, cmd, "--" flag }
+      ' | sort
+}
+
+cmd=${1:-}; old=${2:-}; new=${3:-}
+[ -n "$cmd" ] && [ -n "$old" ] && [ -n "$new" ] || die "usage: $0 {diff|deprecated} <old-ref> <new-ref>"
+git rev-parse --verify --quiet "$old" >/dev/null || die "cannot resolve ref '$old'"
+git rev-parse --verify --quiet "$new" >/dev/null || die "cannot resolve ref '$new'"
+
+case "$cmd" in
+  diff)
+    surface "$old" > /tmp/surface-old.txt
+    surface "$new" > /tmp/surface-new.txt
+    # A reduction that matched nothing is a broken check, not an unchanged
+    # surface. Without this the two look identical: both print no diff.
+    [ -s /tmp/surface-old.txt ] && [ -s /tmp/surface-new.txt ] \
+      || die "surface() matched nothing - the check is broken, do not read the result as clean"
+    diff /tmp/surface-old.txt /tmp/surface-new.txt
+    ;;
+  deprecated)
+    # Only the old side is asserted: an empty new side is a legitimate result
+    # (no deprecated flags anywhere), and an empty old side is normal too, so
+    # the sanity check here is that gen/ resolved at all.
+    # wc, not `grep -q`: grep exits on the first match, which SIGPIPEs the
+    # upstream git show and trips pipefail, failing a check that actually passed.
+    [ "$(gen_at "$old" | wc -l)" -gt 0 ] || die "gen/ is empty at '$old' - the check is broken"
+    deprecated_flags "$old" > /tmp/dep-old.txt
+    deprecated_flags "$new" > /tmp/dep-new.txt
+    comm -13 /tmp/dep-old.txt /tmp/dep-new.txt
+    ;;
+  *) die "unknown check '$cmd' (expected: diff, deprecated)" ;;
+esac