← back to Cli Printing Press
fix(cli): make live-dogfood runner enum-aware and resilient to empty outcomes (#1656)
cda9b43b60469e072832b5883911795b9faeb44f · 2026-05-19 08:44:05 -0700 · Trevin Chow
* fix(cli): make live-dogfood runner enum-aware and resilient to empty outcomes
Three independent fixes for the live dogfood runner, all in the
matrix-builder / acceptance-writer / summary-renderer codepath.
1. Enum-aware happy_path values. `exampleValue` now prefers the first
declared enum value when a param carries spec-level enum constraints,
so `pet find-by-status --status available` runs instead of the
API-rejected `--status example-value`. PII-synthetic shapes still
win to keep browser-sniff captures from leaking customer data into
examples.
2. `--write-acceptance` writes on every outcome. The flag previously
only emitted `phase5-acceptance.json` on PASS, forcing operators to
hand-author the FAIL marker the Phase 5.6 gate also forbids editing.
The runner now writes `status: "pass"` or `status: "fail"`
accordingly; the FAIL marker carries a `failure_summary` block
bucketing failures by category (transport_error, http_4xx, http_5xx,
exit_nonzero, output_mismatch, other) plus the contributing
commands. `phase5_gate.go` already routed `status: "fail"` to the
hold path, so the gate behavior is unchanged.
3. Path Validity N/A for empty matrix. `printDogfoodReport` rendered
`0/0 valid (FAIL)` when SpecPath was present, the check was not
Skipped, and Tested was 0 — cosmetic divide-by-zero misalignment
with the scorecard, which reports 10/10 in the same run. Empty
matrices now render `N/A`, matching the failure aggregator at
`FailedIssues` that already guards on `Tested > 0`.
The phase5-acceptance JSON Schema gains an optional `failure_summary`
definition, drops the unconditional `tests_passed >= 1` floor, and adds
a conditional rule that keeps the floor for `status: "pass"` markers.
Closes #1384
* fix(cli): tighten live-dogfood failure classifier per Greptile review
Address both findings on PR #1656 review of classifyLiveDogfoodFailure:
1. Match "invalid json" / "not json" independently of the "output"
substring so the runner's own literal "invalid JSON" reason strings
bucket as output_mismatch rather than falling through to exit_nonzero.
2. Check "http 4" before "http 5" so a retry-count log mentioning
"http 5" earlier cannot shadow a real 4xx response (e.g.,
"retried http 5 times, status http 404").
Adds TestClassifyLiveDogfoodFailure covering both regressions plus the
transport_error, output+mismatch conjunction, exit_nonzero, and other
branches so future drift trips a targeted assertion instead of the
existing aggregate bucket-sum check in TestRunLiveDogfoodWritesFailMarkerOnFail.
---------
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Files touched
M internal/cli/dogfood.goM internal/cli/dogfood_test.goM internal/cli/schema.goM internal/generator/example_value_test.goM internal/generator/generator.goM internal/pipeline/live_dogfood.goM internal/pipeline/live_dogfood_test.goM internal/pipeline/phase5_gate.goM skills/printing-press/SKILL.mdM testdata/golden/expected/schema-phase5-marker/stdout.txt
Diff
commit cda9b43b60469e072832b5883911795b9faeb44f
Author: Trevin Chow <trevin@trevinchow.com>
Date: Tue May 19 08:44:05 2026 -0700
fix(cli): make live-dogfood runner enum-aware and resilient to empty outcomes (#1656)
* fix(cli): make live-dogfood runner enum-aware and resilient to empty outcomes
Three independent fixes for the live dogfood runner, all in the
matrix-builder / acceptance-writer / summary-renderer codepath.
1. Enum-aware happy_path values. `exampleValue` now prefers the first
declared enum value when a param carries spec-level enum constraints,
so `pet find-by-status --status available` runs instead of the
API-rejected `--status example-value`. PII-synthetic shapes still
win to keep browser-sniff captures from leaking customer data into
examples.
2. `--write-acceptance` writes on every outcome. The flag previously
only emitted `phase5-acceptance.json` on PASS, forcing operators to
hand-author the FAIL marker the Phase 5.6 gate also forbids editing.
The runner now writes `status: "pass"` or `status: "fail"`
accordingly; the FAIL marker carries a `failure_summary` block
bucketing failures by category (transport_error, http_4xx, http_5xx,
exit_nonzero, output_mismatch, other) plus the contributing
commands. `phase5_gate.go` already routed `status: "fail"` to the
hold path, so the gate behavior is unchanged.
3. Path Validity N/A for empty matrix. `printDogfoodReport` rendered
`0/0 valid (FAIL)` when SpecPath was present, the check was not
Skipped, and Tested was 0 — cosmetic divide-by-zero misalignment
with the scorecard, which reports 10/10 in the same run. Empty
matrices now render `N/A`, matching the failure aggregator at
`FailedIssues` that already guards on `Tested > 0`.
The phase5-acceptance JSON Schema gains an optional `failure_summary`
definition, drops the unconditional `tests_passed >= 1` floor, and adds
a conditional rule that keeps the floor for `status: "pass"` markers.
Closes #1384
* fix(cli): tighten live-dogfood failure classifier per Greptile review
Address both findings on PR #1656 review of classifyLiveDogfoodFailure:
1. Match "invalid json" / "not json" independently of the "output"
substring so the runner's own literal "invalid JSON" reason strings
bucket as output_mismatch rather than falling through to exit_nonzero.
2. Check "http 4" before "http 5" so a retry-count log mentioning
"http 5" earlier cannot shadow a real 4xx response (e.g.,
"retried http 5 times, status http 404").
Adds TestClassifyLiveDogfoodFailure covering both regressions plus the
transport_error, output+mismatch conjunction, exit_nonzero, and other
branches so future drift trips a targeted assertion instead of the
existing aggregate bucket-sum check in TestRunLiveDogfoodWritesFailMarkerOnFail.
---------
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
internal/cli/dogfood.go | 15 +++-
internal/cli/dogfood_test.go | 25 ++++++
internal/cli/schema.go | 26 +++++-
internal/generator/example_value_test.go | 53 ++++++++++++
internal/generator/generator.go | 10 +++
internal/pipeline/live_dogfood.go | 95 +++++++++++++++++++++-
internal/pipeline/live_dogfood_test.go | 94 ++++++++++++++++++++-
internal/pipeline/phase5_gate.go | 37 ++++++---
skills/printing-press/SKILL.md | 14 +++-
.../expected/schema-phase5-marker/stdout.txt | 26 +++++-
10 files changed, 367 insertions(+), 28 deletions(-)
diff --git a/internal/cli/dogfood.go b/internal/cli/dogfood.go
index ff470002..f354acac 100644
--- a/internal/cli/dogfood.go
+++ b/internal/cli/dogfood.go
@@ -88,7 +88,7 @@ func newDogfoodCmd() *cobra.Command {
cmd.Flags().BoolVar(&live, "live", false, "Run the Phase 5 live command-tree dogfood matrix")
cmd.Flags().StringVar(&level, "level", "full", "Live dogfood depth: quick or full")
cmd.Flags().DurationVar(&timeout, "timeout", 30*time.Second, "Timeout for each live dogfood test")
- cmd.Flags().StringVar(&writeAcceptance, "write-acceptance", "", "Write phase5-acceptance.json to this path when live dogfood passes")
+ cmd.Flags().StringVar(&writeAcceptance, "write-acceptance", "", "Write phase5-acceptance.json to this path on every outcome (status:pass on success, status:fail with a failure_summary block on failure)")
cmd.Flags().StringVar(&authEnv, "auth-env", "", "Environment variable that proves an API credential was available for the acceptance marker")
cmd.Flags().BoolVar(&allowDestructive, "allow-destructive", false, "Re-enable testing of endpoints classified as destructive-at-auth. Default skips them to prevent runner-credential rotation.")
_ = cmd.MarkFlagRequired("dir")
@@ -133,9 +133,18 @@ func printDogfoodReport(report *pipeline.DogfoodReport) {
pathStatus := "SKIP"
if report.SpecPath != "" && !report.PathCheck.Skipped {
- pathStatus = "PASS"
- if report.PathCheck.Pct < 70 {
+ // An empty matrix (Tested == 0) is cosmetic, not a failure: the
+ // scorecard's Path Validity dimension renders 10/10 in this case, and
+ // the dogfood failure aggregator at FailedIssues guards on Tested > 0
+ // before counting a low-percentage path-validity failure. Render N/A
+ // here to match.
+ switch {
+ case report.PathCheck.Tested == 0:
+ pathStatus = "N/A"
+ case report.PathCheck.Pct < 70:
pathStatus = "FAIL"
+ default:
+ pathStatus = "PASS"
}
}
fmt.Printf("Path Validity: %d/%d valid (%s)\n", report.PathCheck.Valid, report.PathCheck.Tested, pathStatus)
diff --git a/internal/cli/dogfood_test.go b/internal/cli/dogfood_test.go
index 96320db0..4bed3335 100644
--- a/internal/cli/dogfood_test.go
+++ b/internal/cli/dogfood_test.go
@@ -31,6 +31,31 @@ func TestPrintDogfoodReportRespectsSkippedPathCheck(t *testing.T) {
assert.NotContains(t, out, "Path Validity: 0/0 valid (FAIL)")
}
+// TestPrintDogfoodReportRendersEmptyMatrixAsNA covers the cosmetic
+// divide-by-zero in the path-validity renderer when SpecPath is present and
+// the check is not Skipped but Tested is zero (small-surface CLIs whose
+// command tree didn't produce any path-validity matrix entries). The
+// scorecard's Path Validity dim correctly reports 10/10 in the same run, so
+// rendering FAIL here is misleading to a first-time reader.
+func TestPrintDogfoodReportRendersEmptyMatrixAsNA(t *testing.T) {
+ report := &pipeline.DogfoodReport{
+ Dir: t.TempDir(),
+ SpecPath: "petstore.yaml",
+ PathCheck: pipeline.PathCheckResult{
+ Tested: 0,
+ Valid: 0,
+ Skipped: false,
+ },
+ }
+
+ out := captureStdout(t, func() {
+ printDogfoodReport(report)
+ })
+
+ assert.Contains(t, out, "Path Validity: 0/0 valid (N/A)")
+ assert.NotContains(t, out, "Path Validity: 0/0 valid (FAIL)")
+}
+
func TestDogfoodHelpIncludesLiveFlags(t *testing.T) {
cmd := newDogfoodCmd()
cmd.SetArgs([]string{"--help"})
diff --git a/internal/cli/schema.go b/internal/cli/schema.go
index 9ed0088e..95425f7f 100644
--- a/internal/cli/schema.go
+++ b/internal/cli/schema.go
@@ -198,7 +198,7 @@ const phase5MarkerSchemaJSON = `{
"title": "CLI Printing Press phase5-acceptance.json",
"type": "object",
"additionalProperties": false,
- "required": ["schema_version", "api_name", "run_id", "status", "level", "matrix_size", "tests_passed"],
+ "required": ["schema_version", "api_name", "run_id", "status", "level", "matrix_size"],
"properties": {
"schema_version": {"type": "integer", "const": 1},
"api_name": {"type": "string", "minLength": 1},
@@ -208,13 +208,20 @@ const phase5MarkerSchemaJSON = `{
"level": {"type": "string", "enum": ["quick", "full"]},
"matrix_size": {"type": "integer", "minimum": 1},
"tests_total": {"type": "integer", "minimum": 0},
- "tests_passed": {"type": "integer", "minimum": 1},
+ "tests_passed": {"type": "integer", "minimum": 0},
"tests_skipped": {"type": "integer", "minimum": 0},
"tests_failed": {"type": "integer", "minimum": 0},
"completed_at": {"type": "string", "format": "date-time"},
"summary": {"type": "string"},
- "auth_context": {"$ref": "#/$defs/auth_context"}
+ "auth_context": {"$ref": "#/$defs/auth_context"},
+ "failure_summary": {"$ref": "#/$defs/failure_summary"}
},
+ "allOf": [
+ {
+ "if": {"properties": {"status": {"const": "pass"}}, "required": ["status"]},
+ "then": {"required": ["tests_passed"], "properties": {"tests_passed": {"minimum": 1}}}
+ }
+ ],
"$defs": {
"auth_context": {
"type": "object",
@@ -224,6 +231,19 @@ const phase5MarkerSchemaJSON = `{
"api_key_available": {"type": "boolean"},
"browser_session_available": {"type": "boolean"}
}
+ },
+ "failure_summary": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "transport_error": {"type": "integer", "minimum": 0},
+ "http_4xx": {"type": "integer", "minimum": 0},
+ "http_5xx": {"type": "integer", "minimum": 0},
+ "exit_nonzero": {"type": "integer", "minimum": 0},
+ "output_mismatch": {"type": "integer", "minimum": 0},
+ "other": {"type": "integer", "minimum": 0},
+ "commands": {"type": "array", "items": {"type": "string"}}
+ }
}
}
}`
diff --git a/internal/generator/example_value_test.go b/internal/generator/example_value_test.go
index b2306709..1662f33a 100644
--- a/internal/generator/example_value_test.go
+++ b/internal/generator/example_value_test.go
@@ -5,6 +5,7 @@ import (
"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
// TestExampleValueIDRecognition covers the three id-shape clauses: bare `id`,
@@ -79,3 +80,55 @@ func TestExampleValueIDRecognition(t *testing.T) {
})
}
}
+
+// TestExampleValueEnumWinsOverNameHeuristics covers enum-aware placeholder
+// selection. When a param carries enum constraints the API will reject any
+// value outside that set, so enum[0] must beat the name-based branches that
+// would otherwise emit `example-value` / `example-resource` / a UUID.
+func TestExampleValueEnumWinsOverNameHeuristics(t *testing.T) {
+ tests := []struct {
+ name string
+ param spec.Param
+ want string
+ }{
+ // Petstore find-by-status: bare `status` with enum → enum[0], not example-value.
+ {"status string enum picks first", spec.Param{Name: "status", Type: "string", Enum: []string{"available", "pending", "sold"}}, "available"},
+ // Petstore find-by-tags: even array-typed enum params take enum[0] (per
+ // CLI flag parsing — a single value is always a legal subset of the array).
+ {"tags array enum picks first", spec.Param{Name: "tags", Type: "array", Enum: []string{"cat", "dog", "rabbit"}}, "cat"},
+ // Empty/whitespace entries are skipped; first non-empty value wins.
+ {"enum skips empty entries", spec.Param{Name: "mode", Type: "string", Enum: []string{"", " ", "deep"}}, "deep"},
+ // Enum wins over name-shape heuristic. `*Id` would otherwise route to UUID.
+ {"enum beats id-shape", spec.Param{Name: "statusId", Type: "string", Enum: []string{"a", "b"}}, "a"},
+ // Enum wins over the `name`/`title` branch that returns `example-resource`.
+ {"enum beats name branch", spec.Param{Name: "displayName", Type: "string", Enum: []string{"alpha", "beta"}}, "alpha"},
+ // PII-synthetic shapes (asin, card_last4, recipient, address) take
+ // priority over enum: the synthetic placeholder protects browser-sniff
+ // runs from echoing captured customer data, and enum constraints on
+ // those fields are vanishingly rare in practice. If a real API ever
+ // hits this case, the synthetic value is wrong but safe; enum-driven
+ // placeholders would be right but risk re-emitting captured PII.
+ {"synthetic asin beats enum", spec.Param{Name: "asin", Type: "string", Enum: []string{"X1", "X2"}}, "B0EXAMPLE1"},
+ // Empty enum slice is a no-op; fall through to default behavior.
+ {"empty enum falls through", spec.Param{Name: "status", Type: "string", Enum: nil}, "example-value"},
+ // Non-string enums (the OpenAPI parser still populates Enum as []string;
+ // the placeholder doesn't need conversion logic — the printed CLI's
+ // validator handles parsing).
+ {"numeric enum still string-stringified", spec.Param{Name: "priority", Type: "integer", Enum: []string{"1", "2", "3"}}, "1"},
+ }
+
+ // Sanity: confirm the asin synthetic placeholder hasn't moved out from
+ // under the assertion above. If syntheticExampleValue ever stops handling
+ // asin, the test below loses its meaning (would silently become an
+ // enum-wins assertion instead of a synthetic-beats-enum assertion).
+ syntheticASIN, ok := syntheticExampleValue("asin")
+ require.True(t, ok, "syntheticExampleValue must continue to recognize 'asin'; otherwise the synthetic-beats-enum test below loses its meaning")
+ require.Equal(t, "B0EXAMPLE1", syntheticASIN)
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ got := exampleValue(tt.param)
+ assert.Equal(t, tt.want, got)
+ })
+ }
+}
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 66190703..f6d54469 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -4305,6 +4305,16 @@ func exampleValue(p spec.Param) string {
return value
}
+ // Enum-constrained params: the API rejects anything outside the set,
+ // so prefer the first declared value over name-shape heuristics.
+ // This wins over name-based branches because a hypothetical
+ // `status_id: [a,b,c]` enum still requires `a`, not a UUID.
+ for _, v := range p.Enum {
+ if strings.TrimSpace(v) != "" {
+ return v
+ }
+ }
+
nameLower := strings.ToLower(p.Name)
// camelCase `*Id` carries an exclusion fence so bool/numeric params
diff --git a/internal/pipeline/live_dogfood.go b/internal/pipeline/live_dogfood.go
index 2acd0e0e..79908103 100644
--- a/internal/pipeline/live_dogfood.go
+++ b/internal/pipeline/live_dogfood.go
@@ -160,7 +160,13 @@ func RunLiveDogfood(opts LiveDogfoodOptions) (*LiveDogfoodReport, error) {
}
finalizeLiveDogfoodReport(report)
- if report.Verdict == "PASS" && opts.WriteAcceptancePath != "" {
+ // The Phase 5.6 acceptance gate's contract is "marker from the runner on
+ // every outcome": pass → promote, fail → hold-path, missing → "Phase 5
+ // was skipped or not recorded." Writing only on PASS forced operators to
+ // hand-author the FAIL marker, which the SKILL also forbids. Write on
+ // every terminal verdict; phase5_gate.go already routes status:"fail"
+ // to the hold path.
+ if opts.WriteAcceptancePath != "" {
if err := writeLiveDogfoodAcceptance(opts, report); err != nil {
return nil, err
}
@@ -1214,11 +1220,18 @@ func writeLiveDogfoodAcceptance(opts LiveDogfoodOptions, report *LiveDogfoodRepo
authType = "none"
}
+ status := "pass"
+ var failureSummary *Phase5FailureSummary
+ if report.Verdict != "PASS" {
+ status = "fail"
+ failureSummary = summarizeLiveDogfoodFailures(report)
+ }
+
marker := Phase5GateMarker{
SchemaVersion: 1,
APIName: apiName,
RunID: runID,
- Status: "pass",
+ Status: status,
Level: report.Level,
MatrixSize: report.MatrixSize,
TestsPassed: report.Passed,
@@ -1228,6 +1241,7 @@ func writeLiveDogfoodAcceptance(opts LiveDogfoodOptions, report *LiveDogfoodRepo
Type: authType,
APIKeyAvailable: opts.AuthEnv != "" && os.Getenv(opts.AuthEnv) != "",
},
+ FailureSummary: failureSummary,
}
data, err := json.MarshalIndent(marker, "", " ")
if err != nil {
@@ -1242,6 +1256,83 @@ func writeLiveDogfoodAcceptance(opts LiveDogfoodOptions, report *LiveDogfoodRepo
return nil
}
+// summarizeLiveDogfoodFailures groups failed test results by category so the
+// fail-marker carries a one-glance triage hint. Categories mirror the
+// retro's suggested buckets: transport-error, http-4xx, http-5xx,
+// exit-nonzero, output-mismatch, other. Commands lists deduplicated command
+// names that contributed at least one failure.
+func summarizeLiveDogfoodFailures(report *LiveDogfoodReport) *Phase5FailureSummary {
+ if report == nil {
+ return nil
+ }
+ summary := &Phase5FailureSummary{}
+ seen := map[string]bool{}
+ for _, t := range report.Tests {
+ if t.Status != LiveDogfoodStatusFail {
+ continue
+ }
+ switch classifyLiveDogfoodFailure(t) {
+ case "transport_error":
+ summary.TransportError++
+ case "http_4xx":
+ summary.HTTP4xx++
+ case "http_5xx":
+ summary.HTTP5xx++
+ case "exit_nonzero":
+ summary.ExitNonzero++
+ case "output_mismatch":
+ summary.OutputMismatch++
+ default:
+ summary.Other++
+ }
+ if t.Command != "" && !seen[t.Command] {
+ seen[t.Command] = true
+ summary.Commands = append(summary.Commands, t.Command)
+ }
+ }
+ if summary.TransportError == 0 && summary.HTTP4xx == 0 && summary.HTTP5xx == 0 &&
+ summary.ExitNonzero == 0 && summary.OutputMismatch == 0 && summary.Other == 0 {
+ return nil
+ }
+ sort.Strings(summary.Commands)
+ return summary
+}
+
+// classifyLiveDogfoodFailure picks the failure bucket for one test result.
+// The reason string and a small slice of the captured output (already
+// truncated to OutputSample) are the only signals; classification is a
+// best-effort hint, not a contract.
+func classifyLiveDogfoodFailure(t LiveDogfoodTestResult) string {
+ hay := strings.ToLower(t.Reason + " " + t.OutputSample)
+ // 4xx is checked before 5xx: a legitimate 5xx response is unlikely to
+ // also mention "http 4", whereas error strings citing 400/401/403/404
+ // frequently start with digit 4 and would otherwise be shadowed if 5xx
+ // were checked first (e.g., a retry-count log like
+ // "retried http 5 times, status http 404").
+ switch {
+ case strings.Contains(hay, "http 4"):
+ return "http_4xx"
+ case strings.Contains(hay, "http 5"):
+ return "http_5xx"
+ case strings.Contains(hay, "connection refused") ||
+ strings.Contains(hay, "no such host") ||
+ strings.Contains(hay, "timeout") ||
+ strings.Contains(hay, "dial tcp"):
+ return "transport_error"
+ // "invalid json" / "not json" match independently so the runner's own
+ // Reason strings (literal "invalid JSON" at the two emit sites) bucket
+ // here even when neither Reason nor OutputSample contains the word
+ // "output". The "output" + "mismatch" conjunction stays as a separate
+ // match for the schema-mismatch flavor of failure.
+ case strings.Contains(hay, "invalid json") || strings.Contains(hay, "not json") ||
+ (strings.Contains(hay, "output") && strings.Contains(hay, "mismatch")):
+ return "output_mismatch"
+ case t.ExitCode != 0:
+ return "exit_nonzero"
+ }
+ return "other"
+}
+
// resolveLiveDogfoodAcceptanceIdentity finds the marker's api_name, run_id,
// and auth_type. Manifest on disk wins (also yields auth_type); runstate
// fills in when the manifest hasn't been written yet (the pre-promote case
diff --git a/internal/pipeline/live_dogfood_test.go b/internal/pipeline/live_dogfood_test.go
index 3172985c..6c6f24e9 100644
--- a/internal/pipeline/live_dogfood_test.go
+++ b/internal/pipeline/live_dogfood_test.go
@@ -84,7 +84,11 @@ func TestRunLiveDogfoodWritesAcceptanceMarkerOnPass(t *testing.T) {
assert.True(t, validation.Passed, validation.Detail)
}
-func TestRunLiveDogfoodDoesNotWriteAcceptanceMarkerOnFail(t *testing.T) {
+// TestRunLiveDogfoodWritesFailMarkerOnFail covers the inverted contract from
+// issue #1384: --write-acceptance must emit a marker on every outcome, so the
+// Phase 5.6 gate has something to read (pass → promote, fail → hold-path)
+// instead of forcing operators to hand-author the FAIL marker.
+func TestRunLiveDogfoodWritesFailMarkerOnFail(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("test uses a shell script as the fake binary; skip on Windows")
}
@@ -102,8 +106,92 @@ func TestRunLiveDogfoodDoesNotWriteAcceptanceMarkerOnFail(t *testing.T) {
require.Equal(t, "FAIL", report.Verdict, report.Tests)
assert.Greater(t, report.Failed, 0)
- _, statErr := os.Stat(markerPath)
- assert.True(t, os.IsNotExist(statErr), "failed live dogfood must not write an acceptance marker")
+ data, err := os.ReadFile(markerPath)
+ require.NoError(t, err, "failed live dogfood must still write an acceptance marker for Phase 5.6")
+ var marker Phase5GateMarker
+ require.NoError(t, json.Unmarshal(data, &marker))
+ assert.Equal(t, "fail", marker.Status)
+ assert.Equal(t, report.Failed, marker.TestsFailed)
+ require.NotNil(t, marker.FailureSummary, "fail markers must carry a failure_summary block")
+ // The fixture's failing branch produces exit-nonzero results; the
+ // classifier may also bucket some as http_4xx/5xx depending on the
+ // fixture's emitted reason text. Either way, the aggregate failure
+ // count across all buckets must match report.Failed so no failure is
+ // silently dropped.
+ total := marker.FailureSummary.TransportError + marker.FailureSummary.HTTP4xx +
+ marker.FailureSummary.HTTP5xx + marker.FailureSummary.ExitNonzero +
+ marker.FailureSummary.OutputMismatch + marker.FailureSummary.Other
+ assert.Equal(t, report.Failed, total, "failure_summary buckets must account for every failed test")
+ assert.NotEmpty(t, marker.FailureSummary.Commands, "failure_summary must list at least one failing command")
+
+ // The Phase 5 gate must route this marker to the hold path, not pass it.
+ validation := ValidatePhase5Gate(filepath.Dir(markerPath), CLIManifest{APIName: marker.APIName, RunID: marker.RunID, AuthType: "none"})
+ assert.False(t, validation.Passed)
+ assert.Equal(t, "fail", validation.Status)
+}
+
+// TestClassifyLiveDogfoodFailure covers the per-test bucket assignment. The
+// classifier feeds failure_summary triage hints; missing a bucket silently
+// downgrades the operator signal, so each branch needs an explicit fixture.
+// HTTP ordering and the JSON-mismatch fall-through were Greptile findings on
+// the PR introducing this function and are pinned here as regressions.
+func TestClassifyLiveDogfoodFailure(t *testing.T) {
+ cases := []struct {
+ name string
+ in LiveDogfoodTestResult
+ want string
+ }{
+ {
+ name: "http_4xx from reason",
+ in: LiveDogfoodTestResult{Reason: "got HTTP 404 from upstream", ExitCode: 1},
+ want: "http_4xx",
+ },
+ {
+ name: "http_5xx from reason",
+ in: LiveDogfoodTestResult{Reason: "got HTTP 503 from upstream", ExitCode: 1},
+ want: "http_5xx",
+ },
+ {
+ name: "4xx wins when both appear (retry log shadowing case)",
+ in: LiveDogfoodTestResult{Reason: "retried http 5 times, status http 404", ExitCode: 1},
+ want: "http_4xx",
+ },
+ {
+ name: "transport_error on connection refused",
+ in: LiveDogfoodTestResult{Reason: "dial tcp: connection refused", ExitCode: 1},
+ want: "transport_error",
+ },
+ {
+ name: "output_mismatch from bare 'invalid JSON' reason",
+ in: LiveDogfoodTestResult{Reason: "invalid JSON", OutputSample: "<<not json>>", ExitCode: 1},
+ want: "output_mismatch",
+ },
+ {
+ name: "output_mismatch from 'not json' reason without 'output' word",
+ in: LiveDogfoodTestResult{Reason: "response was not JSON", ExitCode: 1},
+ want: "output_mismatch",
+ },
+ {
+ name: "output_mismatch from output+mismatch conjunction",
+ in: LiveDogfoodTestResult{Reason: "output mismatch vs schema", ExitCode: 1},
+ want: "output_mismatch",
+ },
+ {
+ name: "exit_nonzero fall-through",
+ in: LiveDogfoodTestResult{Reason: "unknown failure", ExitCode: 2},
+ want: "exit_nonzero",
+ },
+ {
+ name: "other when nothing matches and exit code is zero",
+ in: LiveDogfoodTestResult{Reason: "weird thing happened", ExitCode: 0},
+ want: "other",
+ },
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ assert.Equal(t, tc.want, classifyLiveDogfoodFailure(tc.in))
+ })
+ }
}
// TestDogfoodEnvVarMatchesEmittedTemplate guards against the runner-side
diff --git a/internal/pipeline/phase5_gate.go b/internal/pipeline/phase5_gate.go
index 84fe0ac7..202c7cbf 100644
--- a/internal/pipeline/phase5_gate.go
+++ b/internal/pipeline/phase5_gate.go
@@ -28,17 +28,32 @@ type Phase5AuthContext struct {
}
type Phase5GateMarker struct {
- SchemaVersion int `json:"schema_version"`
- APIName string `json:"api_name,omitempty"`
- RunID string `json:"run_id,omitempty"`
- Status string `json:"status"`
- Level string `json:"level,omitempty"`
- MatrixSize int `json:"matrix_size,omitempty"`
- TestsPassed int `json:"tests_passed,omitempty"`
- TestsSkipped int `json:"tests_skipped,omitempty"`
- TestsFailed int `json:"tests_failed,omitempty"`
- AuthContext Phase5AuthContext `json:"auth_context,omitzero"`
- SkipReason string `json:"skip_reason,omitempty"`
+ SchemaVersion int `json:"schema_version"`
+ APIName string `json:"api_name,omitempty"`
+ RunID string `json:"run_id,omitempty"`
+ Status string `json:"status"`
+ Level string `json:"level,omitempty"`
+ MatrixSize int `json:"matrix_size,omitempty"`
+ TestsPassed int `json:"tests_passed,omitempty"`
+ TestsSkipped int `json:"tests_skipped,omitempty"`
+ TestsFailed int `json:"tests_failed,omitempty"`
+ AuthContext Phase5AuthContext `json:"auth_context,omitzero"`
+ SkipReason string `json:"skip_reason,omitempty"`
+ FailureSummary *Phase5FailureSummary `json:"failure_summary,omitempty"`
+}
+
+// Phase5FailureSummary groups failed tests by category so a human reviewing
+// a status:"fail" marker can route diagnosis without re-reading the full
+// dogfood-results-v2.json. Populated only when the runner writes a marker
+// on FAIL; absent on PASS markers.
+type Phase5FailureSummary struct {
+ TransportError int `json:"transport_error,omitempty"`
+ HTTP4xx int `json:"http_4xx,omitempty"`
+ HTTP5xx int `json:"http_5xx,omitempty"`
+ ExitNonzero int `json:"exit_nonzero,omitempty"`
+ OutputMismatch int `json:"output_mismatch,omitempty"`
+ Other int `json:"other,omitempty"`
+ Commands []string `json:"commands,omitempty"`
}
type Phase5GateValidation struct {
diff --git a/skills/printing-press/SKILL.md b/skills/printing-press/SKILL.md
index fc28897e..0fbedcab 100644
--- a/skills/printing-press/SKILL.md
+++ b/skills/printing-press/SKILL.md
@@ -2927,8 +2927,10 @@ structured report with pass/fail/skipped counts. Save the JSON report to:
`$PROOFS_DIR/<stamp>-dogfood-results.json`
If the command exits non-zero, inspect the structured failures, fix the CLI, and
-rerun live dogfood. Do not hand-edit `phase5-acceptance.json`; it must come from
-the runner.
+rerun live dogfood. The runner writes `phase5-acceptance.json` on every outcome
+(`status: "pass"` on success, `status: "fail"` with a `failure_summary` block on
+failure), so the Phase 5.6 gate always has a marker to read. Do not hand-edit
+`phase5-acceptance.json`; it must come from the runner.
**Quick check (auto-selected test subset):**
1. `doctor` — auth valid, API reachable.
@@ -3005,7 +3007,7 @@ Write:
`$PROOFS_DIR/<stamp>-fix-<api>-pp-cli-acceptance.md`
-For `Gate: PASS`, also write:
+For every outcome (PASS or FAIL), the runner writes:
`$PROOFS_DIR/phase5-acceptance.json`
@@ -3027,6 +3029,12 @@ For `Gate: PASS`, also write:
}
```
+On `Gate: FAIL` the same path is written with `status: "fail"` and a
+`failure_summary` block grouping failures by category
+(`transport_error` / `http_4xx` / `http_5xx` / `exit_nonzero` /
+`output_mismatch` / `other`) plus the list of contributing commands. The
+Phase 5.6 gate routes this marker to the hold path; do not promote.
+
For `level: "quick"`, `tests_failed` may be `1` only when the Quick Check
threshold still passed (`matrix_size: 6`, `tests_passed >= 5`) and the miss was
not auth or sync related. For `level: "full"`, `tests_failed` must be `0`.
diff --git a/testdata/golden/expected/schema-phase5-marker/stdout.txt b/testdata/golden/expected/schema-phase5-marker/stdout.txt
index c8a1c61b..5c59aff9 100644
--- a/testdata/golden/expected/schema-phase5-marker/stdout.txt
+++ b/testdata/golden/expected/schema-phase5-marker/stdout.txt
@@ -4,7 +4,7 @@
"title": "CLI Printing Press phase5-acceptance.json",
"type": "object",
"additionalProperties": false,
- "required": ["schema_version", "api_name", "run_id", "status", "level", "matrix_size", "tests_passed"],
+ "required": ["schema_version", "api_name", "run_id", "status", "level", "matrix_size"],
"properties": {
"schema_version": {"type": "integer", "const": 1},
"api_name": {"type": "string", "minLength": 1},
@@ -14,13 +14,20 @@
"level": {"type": "string", "enum": ["quick", "full"]},
"matrix_size": {"type": "integer", "minimum": 1},
"tests_total": {"type": "integer", "minimum": 0},
- "tests_passed": {"type": "integer", "minimum": 1},
+ "tests_passed": {"type": "integer", "minimum": 0},
"tests_skipped": {"type": "integer", "minimum": 0},
"tests_failed": {"type": "integer", "minimum": 0},
"completed_at": {"type": "string", "format": "date-time"},
"summary": {"type": "string"},
- "auth_context": {"$ref": "#/$defs/auth_context"}
+ "auth_context": {"$ref": "#/$defs/auth_context"},
+ "failure_summary": {"$ref": "#/$defs/failure_summary"}
},
+ "allOf": [
+ {
+ "if": {"properties": {"status": {"const": "pass"}}, "required": ["status"]},
+ "then": {"required": ["tests_passed"], "properties": {"tests_passed": {"minimum": 1}}}
+ }
+ ],
"$defs": {
"auth_context": {
"type": "object",
@@ -30,6 +37,19 @@
"api_key_available": {"type": "boolean"},
"browser_session_available": {"type": "boolean"}
}
+ },
+ "failure_summary": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "transport_error": {"type": "integer", "minimum": 0},
+ "http_4xx": {"type": "integer", "minimum": 0},
+ "http_5xx": {"type": "integer", "minimum": 0},
+ "exit_nonzero": {"type": "integer", "minimum": 0},
+ "output_mismatch": {"type": "integer", "minimum": 0},
+ "other": {"type": "integer", "minimum": 0},
+ "commands": {"type": "array", "items": {"type": "string"}}
+ }
}
}
}
← 5cb39a69 fix(skills): stop phase 4.95 code review from being skipped
·
back to Cli Printing Press
·
feat(skills): check public library for existing CLI before g 81f07e53 →