[object Object]

← back to Cli Printing Press

fix(cli): short-circuit sync on dry-run sentinel response (#1471)

f8d3ba803f0b483305737ef5259c8e09c3e26c86 · 2026-05-15 20:24:16 -0700 · Trevin Chow

* fix(cli): short-circuit sync on dry-run sentinel response

client.dryRun returns `{"dry_run": true}` instead of a real response
when --dry-run is set. The sync template fell through to
upsertSingleObject against the sentinel, which failed with
"missing id for <resource>" because the sentinel has no items and no id.
Every CLI shipped with a `sync` quickstart blocked shipcheck because
validate-narrative --full-examples auto-appends --dry-run.

Add isDryRunResponse(data) helper and check it right after c.Get in
syncResource. On match, emit a structured sync_dryrun event and return
a zero-count syncResult so the dry-run preview path exits cleanly.

Three golden fixtures updated (generate-golden-api,
generate-sync-walker-api, generate-tier-routing-api) for the new
short-circuit + helper emission.

Closes #1382

* fix(cli): require exact one-key dry_run sentinel in sync short-circuit

Greptile flagged that the previous isDryRunResponse helper unmarshalled
into a single-field struct, which would silently match any real API
response containing a top-level dry_run field alongside other data —
zeroing the sync count without an error. Tighten the check to require
exactly one key in the envelope (dry_run) with a boolean true value.
Three goldens regenerated for the helper body change.

Also drop a ticket-number reference from a test comment per the
project's no-ticket-numbers-in-code-comments rule.

Refs #1382

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Files touched

Diff

commit f8d3ba803f0b483305737ef5259c8e09c3e26c86
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Fri May 15 20:24:16 2026 -0700

    fix(cli): short-circuit sync on dry-run sentinel response (#1471)
    
    * fix(cli): short-circuit sync on dry-run sentinel response
    
    client.dryRun returns `{"dry_run": true}` instead of a real response
    when --dry-run is set. The sync template fell through to
    upsertSingleObject against the sentinel, which failed with
    "missing id for <resource>" because the sentinel has no items and no id.
    Every CLI shipped with a `sync` quickstart blocked shipcheck because
    validate-narrative --full-examples auto-appends --dry-run.
    
    Add isDryRunResponse(data) helper and check it right after c.Get in
    syncResource. On match, emit a structured sync_dryrun event and return
    a zero-count syncResult so the dry-run preview path exits cleanly.
    
    Three golden fixtures updated (generate-golden-api,
    generate-sync-walker-api, generate-tier-routing-api) for the new
    short-circuit + helper emission.
    
    Closes #1382
    
    * fix(cli): require exact one-key dry_run sentinel in sync short-circuit
    
    Greptile flagged that the previous isDryRunResponse helper unmarshalled
    into a single-field struct, which would silently match any real API
    response containing a top-level dry_run field alongside other data —
    zeroing the sync count without an error. Tighten the check to require
    exactly one key in the envelope (dry_run) with a boolean true value.
    Three goldens regenerated for the helper body change.
    
    Also drop a ticket-number reference from a test comment per the
    project's no-ticket-numbers-in-code-comments rule.
    
    Refs #1382
    
    ---------
    
    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 internal/generator/generator_test.go               | 29 +++++++++++++++++
 internal/generator/templates/sync.go.tmpl          | 37 ++++++++++++++++++++++
 .../printing-press-golden/internal/cli/sync.go     | 37 ++++++++++++++++++++++
 .../sync-walker-golden/internal/cli/sync.go        | 37 ++++++++++++++++++++++
 .../tier-routing-golden/internal/cli/sync.go       | 37 ++++++++++++++++++++++
 5 files changed, 177 insertions(+)

diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 96bc93be..fd4fd4d2 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -10318,6 +10318,35 @@ func TestStaleTemplateCoversCommonTimestampFields(t *testing.T) {
 		"pm_stale.go.tmpl must bind a numeric cutoff alongside the RFC 3339 cutoff for integer-typed timestamps")
 }
 
+// TestSyncTemplateShortCircuitsOnDryRunSentinel: client.dryRun returns
+// `{"dry_run": true}` instead of a real response, and the sync loop
+// must detect this with isDryRunResponse and emit a synthetic
+// sync_dryrun event before falling through to upsertSingleObject —
+// otherwise validate-narrative --full-examples (which auto-appends
+// --dry-run) blocks shipcheck on a spurious "missing id for <resource>"
+// error.
+func TestSyncTemplateShortCircuitsOnDryRunSentinel(t *testing.T) {
+	t.Parallel()
+	data, err := os.ReadFile(filepath.Join("templates", "sync.go.tmpl"))
+	require.NoError(t, err)
+	body := string(data)
+
+	assert.Contains(t, body, "func isDryRunResponse(data json.RawMessage) bool",
+		"sync.go.tmpl must define isDryRunResponse helper that detects the client.dryRun sentinel")
+	assert.Contains(t, body, `"sync_dryrun"`,
+		"sync.go.tmpl must emit a sync_dryrun event on the short-circuit path so validate-narrative sees a structured success")
+
+	// Ordering pin: the dry-run check must run BEFORE upsertSingleObject,
+	// otherwise the sentinel reaches the upsert path and triggers a spurious
+	// "missing id for <resource>" error.
+	dryRunCheckIdx := strings.Index(body, "if isDryRunResponse(data)")
+	upsertSingleIdx := strings.Index(body, "if err := upsertSingleObject(db, resource, data)")
+	require.GreaterOrEqual(t, dryRunCheckIdx, 0, "sync.go.tmpl must call isDryRunResponse on the response")
+	require.GreaterOrEqual(t, upsertSingleIdx, 0, "sync.go.tmpl must still call upsertSingleObject for live single-object responses")
+	assert.Less(t, dryRunCheckIdx, upsertSingleIdx,
+		"isDryRunResponse check must run before upsertSingleObject so the dry-run sentinel doesn't fall through to the missing-id error")
+}
+
 // TestSearchTemplateEmitsEmptyJSONEnvelope pins the contract: the
 // generated `search` command in --json (or piped) mode must always emit
 // a valid JSON envelope, including on no matches. Agents pipe stdout
diff --git a/internal/generator/templates/sync.go.tmpl b/internal/generator/templates/sync.go.tmpl
index 1ca1c054..889bb7c9 100644
--- a/internal/generator/templates/sync.go.tmpl
+++ b/internal/generator/templates/sync.go.tmpl
@@ -536,6 +536,19 @@ func syncResource(c interface {
 			return syncResult{Resource: resource, Count: totalCount, Err: fmt.Errorf("fetching %s: %w", resource, err), Duration: time.Since(started)}
 		}
 
+		// Dry-run sentinel: client.dryRun returns `{"dry_run": true}` instead
+		// of a real response when --dry-run is set. The upsert path below
+		// would otherwise fail with "missing id for <resource>" because the
+		// sentinel has no items and no id; emit a synthetic success event so
+		// validate-narrative --full-examples (which auto-appends --dry-run)
+		// sees a clean exit.
+		if isDryRunResponse(data) {
+			if !humanFriendly {
+				fmt.Fprintf(os.Stdout, `{"event":"sync_dryrun","resource":"%s"}`+"\n", resource)
+			}
+			return syncResult{Resource: resource, Count: 0, Duration: time.Since(started)}
+		}
+
 		// Try to extract items from the response.
 		// Strategy: try array first, then common wrapper keys.
 		items, nextCursor, hasMore := extractPageItems(data, pageSize.cursorParam)
@@ -791,6 +804,30 @@ func extractPageItems(data json.RawMessage, cursorParam string) ([]json.RawMessa
 	return nil, "", false
 }
 
+// isDryRunResponse detects the `{"dry_run": true}` sentinel that
+// client.dryRun returns instead of a real API response. The sync loop
+// uses this to short-circuit before the upsert path, which would
+// otherwise fail with "missing id for <resource>" against a sentinel
+// that has no items and no id. The check requires exactly one key
+// (dry_run) so a live API response that happens to include a top-level
+// dry_run field alongside real data isn't misclassified as the
+// sentinel and silently zero out the sync count.
+func isDryRunResponse(data json.RawMessage) bool {
+	var envelope map[string]json.RawMessage
+	if err := json.Unmarshal(data, &envelope); err != nil {
+		return false
+	}
+	if len(envelope) != 1 {
+		return false
+	}
+	raw, ok := envelope["dry_run"]
+	if !ok {
+		return false
+	}
+	var v bool
+	return json.Unmarshal(raw, &v) == nil && v
+}
+
 func isEmptyPageResponse(data json.RawMessage) bool {
 	var direct []json.RawMessage
 	if err := json.Unmarshal(data, &direct); err == nil && !isJSONNull(data) {
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/sync.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/sync.go
index 486fe79e..a830d25a 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/sync.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/sync.go
@@ -475,6 +475,19 @@ func syncResource(c interface {
 			return syncResult{Resource: resource, Count: totalCount, Err: fmt.Errorf("fetching %s: %w", resource, err), Duration: time.Since(started)}
 		}
 
+		// Dry-run sentinel: client.dryRun returns `{"dry_run": true}` instead
+		// of a real response when --dry-run is set. The upsert path below
+		// would otherwise fail with "missing id for <resource>" because the
+		// sentinel has no items and no id; emit a synthetic success event so
+		// validate-narrative --full-examples (which auto-appends --dry-run)
+		// sees a clean exit.
+		if isDryRunResponse(data) {
+			if !humanFriendly {
+				fmt.Fprintf(os.Stdout, `{"event":"sync_dryrun","resource":"%s"}`+"\n", resource)
+			}
+			return syncResult{Resource: resource, Count: 0, Duration: time.Since(started)}
+		}
+
 		// Try to extract items from the response.
 		// Strategy: try array first, then common wrapper keys.
 		items, nextCursor, hasMore := extractPageItems(data, pageSize.cursorParam)
@@ -711,6 +724,30 @@ func extractPageItems(data json.RawMessage, cursorParam string) ([]json.RawMessa
 	return nil, "", false
 }
 
+// isDryRunResponse detects the `{"dry_run": true}` sentinel that
+// client.dryRun returns instead of a real API response. The sync loop
+// uses this to short-circuit before the upsert path, which would
+// otherwise fail with "missing id for <resource>" against a sentinel
+// that has no items and no id. The check requires exactly one key
+// (dry_run) so a live API response that happens to include a top-level
+// dry_run field alongside real data isn't misclassified as the
+// sentinel and silently zero out the sync count.
+func isDryRunResponse(data json.RawMessage) bool {
+	var envelope map[string]json.RawMessage
+	if err := json.Unmarshal(data, &envelope); err != nil {
+		return false
+	}
+	if len(envelope) != 1 {
+		return false
+	}
+	raw, ok := envelope["dry_run"]
+	if !ok {
+		return false
+	}
+	var v bool
+	return json.Unmarshal(raw, &v) == nil && v
+}
+
 func isEmptyPageResponse(data json.RawMessage) bool {
 	var direct []json.RawMessage
 	if err := json.Unmarshal(data, &direct); err == nil && !isJSONNull(data) {
diff --git a/testdata/golden/expected/generate-sync-walker-api/sync-walker-golden/internal/cli/sync.go b/testdata/golden/expected/generate-sync-walker-api/sync-walker-golden/internal/cli/sync.go
index 84011d0b..49e2ae40 100644
--- a/testdata/golden/expected/generate-sync-walker-api/sync-walker-golden/internal/cli/sync.go
+++ b/testdata/golden/expected/generate-sync-walker-api/sync-walker-golden/internal/cli/sync.go
@@ -475,6 +475,19 @@ func syncResource(c interface {
 			return syncResult{Resource: resource, Count: totalCount, Err: fmt.Errorf("fetching %s: %w", resource, err), Duration: time.Since(started)}
 		}
 
+		// Dry-run sentinel: client.dryRun returns `{"dry_run": true}` instead
+		// of a real response when --dry-run is set. The upsert path below
+		// would otherwise fail with "missing id for <resource>" because the
+		// sentinel has no items and no id; emit a synthetic success event so
+		// validate-narrative --full-examples (which auto-appends --dry-run)
+		// sees a clean exit.
+		if isDryRunResponse(data) {
+			if !humanFriendly {
+				fmt.Fprintf(os.Stdout, `{"event":"sync_dryrun","resource":"%s"}`+"\n", resource)
+			}
+			return syncResult{Resource: resource, Count: 0, Duration: time.Since(started)}
+		}
+
 		// Try to extract items from the response.
 		// Strategy: try array first, then common wrapper keys.
 		items, nextCursor, hasMore := extractPageItems(data, pageSize.cursorParam)
@@ -711,6 +724,30 @@ func extractPageItems(data json.RawMessage, cursorParam string) ([]json.RawMessa
 	return nil, "", false
 }
 
+// isDryRunResponse detects the `{"dry_run": true}` sentinel that
+// client.dryRun returns instead of a real API response. The sync loop
+// uses this to short-circuit before the upsert path, which would
+// otherwise fail with "missing id for <resource>" against a sentinel
+// that has no items and no id. The check requires exactly one key
+// (dry_run) so a live API response that happens to include a top-level
+// dry_run field alongside real data isn't misclassified as the
+// sentinel and silently zero out the sync count.
+func isDryRunResponse(data json.RawMessage) bool {
+	var envelope map[string]json.RawMessage
+	if err := json.Unmarshal(data, &envelope); err != nil {
+		return false
+	}
+	if len(envelope) != 1 {
+		return false
+	}
+	raw, ok := envelope["dry_run"]
+	if !ok {
+		return false
+	}
+	var v bool
+	return json.Unmarshal(raw, &v) == nil && v
+}
+
 func isEmptyPageResponse(data json.RawMessage) bool {
 	var direct []json.RawMessage
 	if err := json.Unmarshal(data, &direct); err == nil && !isJSONNull(data) {
diff --git a/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/cli/sync.go b/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/cli/sync.go
index cfa2ca75..18fefc85 100644
--- a/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/cli/sync.go
+++ b/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/cli/sync.go
@@ -449,6 +449,19 @@ func syncResource(c interface {
 			return syncResult{Resource: resource, Count: totalCount, Err: fmt.Errorf("fetching %s: %w", resource, err), Duration: time.Since(started)}
 		}
 
+		// Dry-run sentinel: client.dryRun returns `{"dry_run": true}` instead
+		// of a real response when --dry-run is set. The upsert path below
+		// would otherwise fail with "missing id for <resource>" because the
+		// sentinel has no items and no id; emit a synthetic success event so
+		// validate-narrative --full-examples (which auto-appends --dry-run)
+		// sees a clean exit.
+		if isDryRunResponse(data) {
+			if !humanFriendly {
+				fmt.Fprintf(os.Stdout, `{"event":"sync_dryrun","resource":"%s"}`+"\n", resource)
+			}
+			return syncResult{Resource: resource, Count: 0, Duration: time.Since(started)}
+		}
+
 		// Try to extract items from the response.
 		// Strategy: try array first, then common wrapper keys.
 		items, nextCursor, hasMore := extractPageItems(data, pageSize.cursorParam)
@@ -685,6 +698,30 @@ func extractPageItems(data json.RawMessage, cursorParam string) ([]json.RawMessa
 	return nil, "", false
 }
 
+// isDryRunResponse detects the `{"dry_run": true}` sentinel that
+// client.dryRun returns instead of a real API response. The sync loop
+// uses this to short-circuit before the upsert path, which would
+// otherwise fail with "missing id for <resource>" against a sentinel
+// that has no items and no id. The check requires exactly one key
+// (dry_run) so a live API response that happens to include a top-level
+// dry_run field alongside real data isn't misclassified as the
+// sentinel and silently zero out the sync count.
+func isDryRunResponse(data json.RawMessage) bool {
+	var envelope map[string]json.RawMessage
+	if err := json.Unmarshal(data, &envelope); err != nil {
+		return false
+	}
+	if len(envelope) != 1 {
+		return false
+	}
+	raw, ok := envelope["dry_run"]
+	if !ok {
+		return false
+	}
+	var v bool
+	return json.Unmarshal(raw, &v) == nil && v
+}
+
 func isEmptyPageResponse(data json.RawMessage) bool {
 	var direct []json.RawMessage
 	if err := json.Unmarshal(data, &direct); err == nil && !isJSONNull(data) {

← 5f43de50 feat(cli): add ElevenLabs generation support (#1307)  ·  back to Cli Printing Press  ·  chore(main): release 4.8.0 (#1491) 6fc98d75 →