← back to Cli Printing Press
fix(cli): skip dogfood --live error_path probe for mutating commands (#1225)
f1246ee145ea765f8ee66714e35ad14b6a859f4e · 2026-05-12 11:10:33 -0700 · Trevin Chow
* fix(cli): skip dogfood --live error_path probe for mutating commands
The dogfood --live error_path block ran `<verb> __printing_press_invalid__`
against the live API for mutating commands, with no --dry-run injection.
APIs that accept arbitrary string fields (e.g., tag names, labels, notes)
accepted the sentinel as legitimate input, creating real resources with
no rollback path.
Gate the probe on `mutating`: when true, emit a Skip with a typed reason
mirroring the existing error_path_real skip pattern. The non-mutating
branch is functionally unchanged; the inner `!mutating` guard on
isSearch is dropped because the outer gate short-circuits.
Fixes #1219
* test(cli): add argv-log defense-in-depth for dry-run-fixture mutator skip
Mirrors the argv-log assertion added to the rich-fixture mutator skip
test in the previous commit. Extends the dry-run fixture script with the
same PRINTING_PRESS_TEST_ARGV_LOG side channel setupRichFixture already
uses; the new test sets the env var and asserts the binary was never
invoked with the invalid-id sentinel.
The fixture's existing `exit 99 — matrix should have skipped` handler
already catches regression via the Status / Args / ExitCode assertions,
but stating the "no live invocation" invariant directly removes the
implicit structural-side-effect chain.
Refs #1219
Files touched
M internal/pipeline/live_dogfood.goM internal/pipeline/live_dogfood_test.go
Diff
commit f1246ee145ea765f8ee66714e35ad14b6a859f4e
Author: Trevin Chow <trevin@trevinchow.com>
Date: Tue May 12 11:10:33 2026 -0700
fix(cli): skip dogfood --live error_path probe for mutating commands (#1225)
* fix(cli): skip dogfood --live error_path probe for mutating commands
The dogfood --live error_path block ran `<verb> __printing_press_invalid__`
against the live API for mutating commands, with no --dry-run injection.
APIs that accept arbitrary string fields (e.g., tag names, labels, notes)
accepted the sentinel as legitimate input, creating real resources with
no rollback path.
Gate the probe on `mutating`: when true, emit a Skip with a typed reason
mirroring the existing error_path_real skip pattern. The non-mutating
branch is functionally unchanged; the inner `!mutating` guard on
isSearch is dropped because the outer gate short-circuits.
Fixes #1219
* test(cli): add argv-log defense-in-depth for dry-run-fixture mutator skip
Mirrors the argv-log assertion added to the rich-fixture mutator skip
test in the previous commit. Extends the dry-run fixture script with the
same PRINTING_PRESS_TEST_ARGV_LOG side channel setupRichFixture already
uses; the new test sets the env var and asserts the binary was never
invoked with the invalid-id sentinel.
The fixture's existing `exit 99 — matrix should have skipped` handler
already catches regression via the Status / Args / ExitCode assertions,
but stating the "no live invocation" invariant directly removes the
implicit structural-side-effect chain.
Refs #1219
---
internal/pipeline/live_dogfood.go | 101 ++++++++++++++++++---------------
internal/pipeline/live_dogfood_test.go | 62 +++++++++++++++++---
2 files changed, 109 insertions(+), 54 deletions(-)
diff --git a/internal/pipeline/live_dogfood.go b/internal/pipeline/live_dogfood.go
index c2e9ebde..8fc35106 100644
--- a/internal/pipeline/live_dogfood.go
+++ b/internal/pipeline/live_dogfood.go
@@ -38,6 +38,7 @@ const (
// across the matrix builder, the flag help text, and the test fixtures.
const reasonDestructiveAtAuth = "destructive-at-auth"
const reasonMutatingDryRunOnly = "mutating command dry-run only"
+const reasonMutatingErrorPath = "mutating command; error_path would call live API without --dry-run"
const reasonNoLiveSignal = "no live happy/json pass; credential-unavailable skips cannot certify acceptance"
const reasonUnavailableRunnerCredentials = "unavailable for runner credentials"
@@ -721,58 +722,66 @@ func runLiveDogfoodCommand(command liveDogfoodCommand, ctx resolveCtx) []LiveDog
}
if liveDogfoodCommandTakesArg(command.Help) {
- flagNames := extractFlagNames(command.Help)
- hasQueryFlag := slices.Contains(flagNames, "query")
- // Search-shape strategy is suppressed for mutating leaves so a
- // `delete --query=...` mass-delete is never probed with the
- // invalid-token sentinel against the live API.
- isSearch := commandSupportsSearch(command.Help) && !mutating
- suppliedJSON := slices.Contains(flagNames, "json")
-
- var errorArgs []string
- if isSearch {
- errorArgs = append([]string{}, command.Path...)
- if hasQueryFlag {
- errorArgs = append(errorArgs, "--query", "__printing_press_invalid__")
+ if mutating {
+ // Mutating commands cannot run the error_path probe safely: the
+ // __printing_press_invalid__ sentinel is sent as a real argument
+ // and many APIs accept arbitrary string fields (tag names, labels,
+ // notes), turning the probe into a real create/update/delete with
+ // no rollback. --dry-run injection is the happy_path-only safety
+ // net; for the error_path we skip outright, mirroring how
+ // error_path_real already skips below.
+ results = append(results, skippedLiveDogfoodResult(commandName, LiveDogfoodTestError, reasonMutatingErrorPath))
+ } else {
+ flagNames := extractFlagNames(command.Help)
+ hasQueryFlag := slices.Contains(flagNames, "query")
+ isSearch := commandSupportsSearch(command.Help)
+ suppliedJSON := slices.Contains(flagNames, "json")
+
+ var errorArgs []string
+ if isSearch {
+ errorArgs = append([]string{}, command.Path...)
+ if hasQueryFlag {
+ errorArgs = append(errorArgs, "--query", "__printing_press_invalid__")
+ } else {
+ errorArgs = append(errorArgs, "__printing_press_invalid__")
+ }
+ if suppliedJSON {
+ errorArgs = appendJSONArg(errorArgs)
+ }
} else {
- errorArgs = append(errorArgs, "__printing_press_invalid__")
- }
- if suppliedJSON {
- errorArgs = appendJSONArg(errorArgs)
+ errorArgs = append(append([]string{}, command.Path...), "__printing_press_invalid__")
}
- } else {
- errorArgs = append(append([]string{}, command.Path...), "__printing_press_invalid__")
- }
- errorRun := runLiveDogfoodProcess(ctx.binaryPath, ctx.cliDir, errorArgs, ctx.timeout)
- errorResult := liveDogfoodResult(commandName, LiveDogfoodTestError, errorArgs, errorRun)
-
- if isSearch {
- // Real-world feed/content APIs return recent items as a fallback
- // for unmatched queries, so non-empty results under exit 0 are
- // not a failure signal. The only fail mode is invalid JSON when
- // the caller asked for --json.
- switch {
- case errorRun.exitCode != 0:
- errorResult.Status = LiveDogfoodStatusPass
- errorResult.Reason = ""
- case suppliedJSON && !json.Valid([]byte(errorRun.stdout)):
- errorResult.Status = LiveDogfoodStatusFail
- errorResult.Reason = "invalid JSON under --json"
- default:
- errorResult.Status = LiveDogfoodStatusPass
- errorResult.Reason = ""
- }
- } else {
- if errorRun.exitCode != 0 {
- errorResult.Status = LiveDogfoodStatusPass
- errorResult.Reason = ""
+ errorRun := runLiveDogfoodProcess(ctx.binaryPath, ctx.cliDir, errorArgs, ctx.timeout)
+ errorResult := liveDogfoodResult(commandName, LiveDogfoodTestError, errorArgs, errorRun)
+
+ if isSearch {
+ // Real-world feed/content APIs return recent items as a fallback
+ // for unmatched queries, so non-empty results under exit 0 are
+ // not a failure signal. The only fail mode is invalid JSON when
+ // the caller asked for --json.
+ switch {
+ case errorRun.exitCode != 0:
+ errorResult.Status = LiveDogfoodStatusPass
+ errorResult.Reason = ""
+ case suppliedJSON && !json.Valid([]byte(errorRun.stdout)):
+ errorResult.Status = LiveDogfoodStatusFail
+ errorResult.Reason = "invalid JSON under --json"
+ default:
+ errorResult.Status = LiveDogfoodStatusPass
+ errorResult.Reason = ""
+ }
} else {
- errorResult.Status = LiveDogfoodStatusFail
- errorResult.Reason = "expected non-zero exit for invalid argument"
+ if errorRun.exitCode != 0 {
+ errorResult.Status = LiveDogfoodStatusPass
+ errorResult.Reason = ""
+ } else {
+ errorResult.Status = LiveDogfoodStatusFail
+ errorResult.Reason = "expected non-zero exit for invalid argument"
+ }
}
+ results = append(results, errorResult)
}
- results = append(results, errorResult)
} else {
results = append(results, skippedLiveDogfoodResult(commandName, LiveDogfoodTestError, "no positional argument"))
}
diff --git a/internal/pipeline/live_dogfood_test.go b/internal/pipeline/live_dogfood_test.go
index 66ea6632..bfd91a44 100644
--- a/internal/pipeline/live_dogfood_test.go
+++ b/internal/pipeline/live_dogfood_test.go
@@ -1948,18 +1948,27 @@ func TestRunLiveDogfoodSearchErrorPathInvalidJSON(t *testing.T) {
}
func TestRunLiveDogfoodSearchErrorPathMutationFallthrough(t *testing.T) {
- dir, binaryName, _ := setupRichFixture(t)
+ dir, binaryName, argvLogPath := setupRichFixture(t)
report := runRichFixtureMatrix(t, dir, binaryName)
- // widgets delete has no --query flag and no <query> positional, so
- // commandSupportsSearch returns false. Even if it had --query (it
- // doesn't), the mutating-leaf deny-list (delete is in mutatingVerbs)
- // would still suppress search-shape and route to the existing
- // non-zero-required strategy. Fixture exit 2 → Pass.
+ // widgets delete is a mutating leaf (in mutatingVerbs). The error_path
+ // probe is skipped without invoking the binary so that APIs which would
+ // accept __printing_press_invalid__ as a real id (and queue or perform
+ // the deletion) cannot mutate live data.
got := findResultByCommandKind(report, "widgets delete", LiveDogfoodTestError)
require.NotNil(t, got)
- assert.Equal(t, LiveDogfoodStatusPass, got.Status, got.Reason)
- assert.Equal(t, 2, got.ExitCode)
+ assert.Equal(t, LiveDogfoodStatusSkip, got.Status, got.Reason)
+ assert.Equal(t, reasonMutatingErrorPath, got.Reason)
+ assert.Empty(t, got.Args, "skipped error_path must not include executable mutation args")
+ assert.Equal(t, 0, got.ExitCode, "skipped error_path must not record a real exit code")
+
+ // Defense-in-depth: the binary must not have been invoked with the
+ // invalid-id sentinel. Status=Skip alone is structurally distinct from
+ // a Pass that ran the probe, but a direct argv-log check makes the
+ // "no live invocation" invariant explicit.
+ lines := readArgvLog(t, argvLogPath)
+ assert.Equal(t, 0, countArgvLines(lines, "delete", "__printing_press_invalid__"),
+ "error_path probe must not invoke the binary for a mutating command")
}
// writeLiveDogfoodDryRunFixture builds a CLI binary that exposes three
@@ -1988,6 +1997,12 @@ func writeLiveDogfoodDryRunFixture(t *testing.T) (dir string, binaryName string)
script := `#!/bin/sh
set -u
+# Argv logging side channel — same convention as setupRichFixture. Tests
+# that don't set PRINTING_PRESS_TEST_ARGV_LOG see no behavior change.
+if [ -n "${PRINTING_PRESS_TEST_ARGV_LOG:-}" ]; then
+ printf '%s\n' "$*" >> "$PRINTING_PRESS_TEST_ARGV_LOG"
+fi
+
if [ "$1" = "agent-context" ]; then
cat <<'JSON'
{
@@ -2299,6 +2314,37 @@ func TestRunLiveDogfoodErrorPathRealSkipMatchesHappyPathReason(t *testing.T) {
"resolve-skipped reason should surface the list-companion gap")
}
+func TestRunLiveDogfoodSkipsErrorPathForMutatorWithDryRun(t *testing.T) {
+ if runtime.GOOS == "windows" {
+ t.Skip("test uses a shell script as the fake binary; skip on Windows")
+ }
+ argvLogPath := filepath.Join(t.TempDir(), "argv.log")
+ t.Setenv("PRINTING_PRESS_TEST_ARGV_LOG", argvLogPath)
+ dir, binaryName := writeLiveDogfoodDryRunFixture(t)
+ report := runDryRunFixtureMatrix(t, dir, binaryName)
+
+ // widgets update <id> is a mutator that advertises --dry-run and takes a
+ // positional argument. The fixture is wired to exit 99 ("matrix should
+ // have skipped") if invoked. With the fix the error_path probe must
+ // skip outright instead of running `widgets update __printing_press_invalid__`
+ // against the live API — even though --dry-run could be injected, the
+ // error_path's invalid-argument semantics are not compatible with a
+ // dry-run preview, so the safe action is to skip.
+ got := findResultByCommandKind(report, "widgets update", LiveDogfoodTestError)
+ require.NotNil(t, got, "expected widgets update error_path result in matrix")
+ assert.Equal(t, LiveDogfoodStatusSkip, got.Status, got.Reason)
+ assert.Equal(t, reasonMutatingErrorPath, got.Reason)
+ assert.Empty(t, got.Args, "skipped error_path must not include executable mutation args")
+
+ // Defense-in-depth: assert the binary was never invoked with the
+ // invalid-id sentinel. The exit-99 sentinel in the fixture already
+ // catches regression via the Status/Args/ExitCode assertions, but
+ // stating the "no live invocation" invariant directly is clearer.
+ lines := readArgvLog(t, argvLogPath)
+ assert.Equal(t, 0, countArgvLines(lines, "update", "__printing_press_invalid__"),
+ "error_path probe must not invoke the binary for a mutating command")
+}
+
// TestRunLiveDogfoodErrorPathRealReportContribution locks in the matrix
// counters so the new test kind threads through finalizeLiveDogfoodReport
// without weakening the verdict math (which counts by Status, not by Kind).
← a169ca49 chore(main): release 4.5.1 (#1170)
·
back to Cli Printing Press
·
fix(cli): default Accept to application/json instead of */* e40389ef →