[object Object]

← back to Cli Printing Press

fix(cli): emit structured cache_warning JSON on auto-refresh failure (#1632)

187b69928fe8a11d6c045d3fbc9bb63016296c02 · 2026-05-18 13:00:42 -0700 · Trevin Chow

* fix(cli): emit structured cache_warning JSON on auto-refresh failure

When autoRefreshIfStale's bounded refresh hits an upstream error and the
command proceeds with the stale cache, the generator now emits a
one-line JSON event to stderr alongside the existing prose warning:

  {"event":"cache_warning","reason":"refresh_failed","resources":["homes"],"error":"..."}

meta.freshness already carries this signal in --json output for commands
that wrap with wrapWithProvenance, but novel commands that build their
own response shape (and most agent-facing fan-outs to printed CLIs) only
saw the prose warning on stderr. Agents reading the stdout stream had no
parseable way to know subsequent rows were served from a cache that
doesn't reflect their requested filters.

The new emitter sits next to the prose warning so both stay aligned; the
prose stays for humans, the JSON line gives agents a single grep-able
signal that matches the established sync_warning/sync_error vocabulary.
The matching auto_refresh_test.go.tmpl exercises the emitter shape in
every generated CLI with cache enabled.

Closes #1263

* fix(cli): panic-safe captureStderr + generator assert for auto_refresh_test.go

Addresses Greptile findings on #1632:
- P1: captureStderr lacked deferred cleanup, so a panic in fn() would leak
  the io.ReadAll goroutine and hang the test binary. Add a defer that
  closes the write end of the pipe and restores os.Stderr, leaving the
  happy-path explicit calls as the primary path.
- P2: TestGenerateFreshnessHelperEmitted now asserts auto_refresh_test.go
  is emitted with the expected helpers and snippets. Golden fixtures have
  no cache-enabled case, so without this a Go syntax error in the
  template would only surface when a customer runs go build.

---------

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

Files touched

Diff

commit 187b69928fe8a11d6c045d3fbc9bb63016296c02
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Mon May 18 13:00:42 2026 -0700

    fix(cli): emit structured cache_warning JSON on auto-refresh failure (#1632)
    
    * fix(cli): emit structured cache_warning JSON on auto-refresh failure
    
    When autoRefreshIfStale's bounded refresh hits an upstream error and the
    command proceeds with the stale cache, the generator now emits a
    one-line JSON event to stderr alongside the existing prose warning:
    
      {"event":"cache_warning","reason":"refresh_failed","resources":["homes"],"error":"..."}
    
    meta.freshness already carries this signal in --json output for commands
    that wrap with wrapWithProvenance, but novel commands that build their
    own response shape (and most agent-facing fan-outs to printed CLIs) only
    saw the prose warning on stderr. Agents reading the stdout stream had no
    parseable way to know subsequent rows were served from a cache that
    doesn't reflect their requested filters.
    
    The new emitter sits next to the prose warning so both stay aligned; the
    prose stays for humans, the JSON line gives agents a single grep-able
    signal that matches the established sync_warning/sync_error vocabulary.
    The matching auto_refresh_test.go.tmpl exercises the emitter shape in
    every generated CLI with cache enabled.
    
    Closes #1263
    
    * fix(cli): panic-safe captureStderr + generator assert for auto_refresh_test.go
    
    Addresses Greptile findings on #1632:
    - P1: captureStderr lacked deferred cleanup, so a panic in fn() would leak
      the io.ReadAll goroutine and hang the test binary. Add a defer that
      closes the write end of the pipe and restores os.Stderr, leaving the
      happy-path explicit calls as the primary path.
    - P2: TestGenerateFreshnessHelperEmitted now asserts auto_refresh_test.go
      is emitted with the expected helpers and snippets. Golden fixtures have
      no cache-enabled case, so without this a Go syntax error in the
      template would only surface when a customer runs go build.
    
    ---------
    
    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 internal/generator/generator.go                    |  3 +
 internal/generator/generator_test.go               | 28 +++++++
 internal/generator/templates/auto_refresh.go.tmpl  | 26 ++++++
 .../generator/templates/auto_refresh_test.go.tmpl  | 98 ++++++++++++++++++++++
 4 files changed, 155 insertions(+)

diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 47f03fbf..607a9056 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -1573,6 +1573,9 @@ func (g *Generator) renderOptionalSupportFiles() error {
 		if err := g.renderTemplate("auto_refresh.go.tmpl", filepath.Join("internal", "cli", "auto_refresh.go"), autoRefreshData); err != nil {
 			return fmt.Errorf("rendering auto_refresh: %w", err)
 		}
+		if err := g.renderTemplate("auto_refresh_test.go.tmpl", filepath.Join("internal", "cli", "auto_refresh_test.go"), autoRefreshData); err != nil {
+			return fmt.Errorf("rendering auto_refresh_test: %w", err)
+		}
 	}
 
 	// Emit the git-backed share package only when explicitly enabled and
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 714c3067..e93902ac 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -314,6 +314,14 @@ func TestGenerateFreshnessHelperEmitted(t *testing.T) {
 		"func ensureFreshForResources(",
 		"func ensureFreshForCommand(",
 		"func runAutoRefresh(",
+		// Refresh failure must emit a structured JSON event to stderr so
+		// agents reading novel-command output that bypasses
+		// wrapWithProvenance still see a parseable degraded-state signal
+		// (issue #1263). The prose warning remains for humans.
+		"func emitCacheRefreshFailedEvent(",
+		`emitCacheRefreshFailedEvent(resources, err)`,
+		`"cache_warning"`,
+		`"refresh_failed"`,
 		`"freshness-pp-cli dashboard": {`,
 		`"items",`,
 		`envOptOut := "FRESHNESS_NO_AUTO_REFRESH"`,
@@ -326,6 +334,26 @@ func TestGenerateFreshnessHelperEmitted(t *testing.T) {
 	require.NotEqual(t, -1, openStoreIndex, "auto_refresh.go must open the store after opt-out checks")
 	assert.Less(t, optOutIndex, openStoreIndex, "env opt-out must be checked before opening/migrating the store")
 
+	// auto_refresh_test.go covers the structured cache_warning emitter so a
+	// Go syntax error in auto_refresh_test.go.tmpl is caught at generation
+	// time rather than propagating silently to every customer CLI (the
+	// golden suite has no cache-enabled case, so the template is otherwise
+	// never compiled by scripts/golden.sh verify).
+	autoRefreshTestPath := filepath.Join(outputDir, "internal", "cli", "auto_refresh_test.go")
+	testData, err := os.ReadFile(autoRefreshTestPath)
+	require.NoError(t, err, "auto_refresh_test.go must be emitted when cache is enabled")
+	testSrc := string(testData)
+	for _, snippet := range []string{
+		"func captureStderr(t *testing.T, fn func()) []byte",
+		"func TestEmitCacheRefreshFailedEvent(t *testing.T)",
+		"func TestEmitCacheRefreshFailedEvent_NilResources(t *testing.T)",
+		`emitCacheRefreshFailedEvent([]string{"items"}`,
+		`"cache_warning"`,
+		`"refresh_failed"`,
+	} {
+		assert.Contains(t, testSrc, snippet, "auto_refresh_test.go missing %q", snippet)
+	}
+
 	dataSource, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "data_source.go"))
 	require.NoError(t, err)
 	assert.NotContains(t, string(dataSource), "freshness_checked",
diff --git a/internal/generator/templates/auto_refresh.go.tmpl b/internal/generator/templates/auto_refresh.go.tmpl
index 3813ef59..285c726b 100644
--- a/internal/generator/templates/auto_refresh.go.tmpl
+++ b/internal/generator/templates/auto_refresh.go.tmpl
@@ -5,6 +5,7 @@ package cli
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
 	"strings"
@@ -136,6 +137,7 @@ func autoRefreshIfStale(ctx context.Context, flags *rootFlags, resources []strin
 	meta.Ran = true
 	if err := runAutoRefresh(refreshCtx, flags, db, resources); err != nil {
 		fmt.Fprintf(os.Stderr, "warning: using stale {{.Name}}-pp-cli cache (refresh failed: %v)\n", err)
+		emitCacheRefreshFailedEvent(resources, err)
 		meta.Reason = "refresh_failed"
 		meta.Error = err.Error()
 		return meta
@@ -144,6 +146,30 @@ func autoRefreshIfStale(ctx context.Context, flags *rootFlags, resources []strin
 	return meta
 }
 
+// emitCacheRefreshFailedEvent writes a structured one-line JSON event to
+// stderr alongside the prose warning so agents have a parseable signal
+// that subsequent results are being served from a stale cache. Novel
+// commands that bypass wrapWithProvenance (so meta.freshness never
+// reaches stdout) would otherwise leave agents with only the prose
+// warning. Mirrors the sync_warning shape from sync.go so the same
+// event vocabulary covers both refresh-time and pre-run paths.
+func emitCacheRefreshFailedEvent(resources []string, err error) {
+	payload := struct {
+		Event     string   `json:"event"`
+		Reason    string   `json:"reason"`
+		Resources []string `json:"resources,omitempty"`
+		Error     string   `json:"error"`
+	}{
+		Event:     "cache_warning",
+		Reason:    "refresh_failed",
+		Resources: resources,
+		Error:     err.Error(),
+	}
+	if out, mErr := json.Marshal(payload); mErr == nil {
+		fmt.Fprintln(os.Stderr, string(out))
+	}
+}
+
 // ensureFreshForResources lets hand-authored commands participate in the same
 // freshness hook as generated resource commands. Custom commands should call
 // this before reading from the store, then use wrapWithProvenance for JSON
diff --git a/internal/generator/templates/auto_refresh_test.go.tmpl b/internal/generator/templates/auto_refresh_test.go.tmpl
new file mode 100644
index 00000000..aaf17261
--- /dev/null
+++ b/internal/generator/templates/auto_refresh_test.go.tmpl
@@ -0,0 +1,98 @@
+// Copyright {{currentYear}} {{.Owner}}. Licensed under Apache-2.0. See LICENSE.
+// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+
+package cli
+
+import (
+	"bytes"
+	"encoding/json"
+	"errors"
+	"io"
+	"os"
+	"strings"
+	"testing"
+)
+
+// captureStderr returns the bytes written to os.Stderr while fn runs. Used
+// to assert the structured cache_warning event without going through a full
+// command invocation. Not safe for t.Parallel — swaps the process-global
+// os.Stderr — so callers in this file run serially.
+func captureStderr(t *testing.T, fn func()) []byte {
+	t.Helper()
+	origStderr := os.Stderr
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("creating stderr pipe: %v", err)
+	}
+	os.Stderr = w
+	done := make(chan []byte, 1)
+	go func() {
+		data, _ := io.ReadAll(r)
+		done <- data
+	}()
+	defer func() {
+		_ = w.Close() // no-op in the happy path; unblocks reader on panic
+		os.Stderr = origStderr
+	}()
+	fn()
+	_ = w.Close()
+	os.Stderr = origStderr
+	return <-done
+}
+
+// TestEmitCacheRefreshFailedEvent verifies that a refresh failure produces a
+// one-line structured JSON event on stderr alongside the prose warning so
+// novel commands that bypass wrapWithProvenance still surface a parseable
+// degraded-state signal to agents (issue #1263).
+func TestEmitCacheRefreshFailedEvent(t *testing.T) {
+	stderr := captureStderr(t, func() {
+		emitCacheRefreshFailedEvent([]string{"items"}, errors.New("HTTP 400: missing 'al'"))
+	})
+
+	// One JSON object per line. The prose warning is emitted at a separate
+	// call site so this capture only sees the structured event.
+	lines := bytes.Split(bytes.TrimRight(stderr, "\n"), []byte("\n"))
+	if len(lines) != 1 {
+		t.Fatalf("expected exactly one stderr line, got %d:\n%s", len(lines), string(stderr))
+	}
+
+	var payload struct {
+		Event     string   `json:"event"`
+		Reason    string   `json:"reason"`
+		Resources []string `json:"resources"`
+		Error     string   `json:"error"`
+	}
+	if err := json.Unmarshal(lines[0], &payload); err != nil {
+		t.Fatalf("stderr line not valid JSON: %v\nline: %s", err, string(lines[0]))
+	}
+	if payload.Event != "cache_warning" {
+		t.Errorf("event = %q, want %q", payload.Event, "cache_warning")
+	}
+	if payload.Reason != "refresh_failed" {
+		t.Errorf("reason = %q, want %q", payload.Reason, "refresh_failed")
+	}
+	if len(payload.Resources) != 1 || payload.Resources[0] != "items" {
+		t.Errorf("resources = %v, want [items]", payload.Resources)
+	}
+	if !strings.Contains(payload.Error, "HTTP 400") {
+		t.Errorf("error = %q, expected to contain %q", payload.Error, "HTTP 400")
+	}
+}
+
+// TestEmitCacheRefreshFailedEvent_NilResources verifies the emitter handles
+// a nil resources slice without panicking and omits the field from JSON
+// (the omitempty tag covers nil and empty slice equivalently).
+func TestEmitCacheRefreshFailedEvent_NilResources(t *testing.T) {
+	stderr := captureStderr(t, func() {
+		emitCacheRefreshFailedEvent(nil, errors.New("boom"))
+	})
+
+	line := bytes.TrimSpace(stderr)
+	var payload map[string]any
+	if err := json.Unmarshal(line, &payload); err != nil {
+		t.Fatalf("stderr line not valid JSON: %v\nline: %s", err, string(line))
+	}
+	if _, ok := payload["resources"]; ok {
+		t.Errorf("resources key should be omitted on nil slice, got: %v", payload)
+	}
+}

← e798f671 fix(cli): route Swagger 2.0 specs through openapi2conv to av  ·  back to Cli Printing Press  ·  feat(cli): add sync --path-context KEY=VAL runtime override 766354e6 →