← back to Cli Printing Press
fix(cli): make dogfood acceptance marker manifest read best-effort (#1179)
ee13fb1611755cc2964a506b03bf614a44d190eb · 2026-05-12 02:11:21 -0700 · Trevin Chow
* fix(cli): make dogfood acceptance marker manifest read best-effort
Phase 5 dogfood writes phase5-acceptance.json before `lock promote`
creates .printing-press.json, so requiring the manifest at acceptance
time errored out every successful generation. Make the read tolerate a
missing file: when the manifest exists, identity and auth_type still
flow into the marker; when it does not, the marker carries the dogfood
run's own state (matrix size, pass/fail/skip counts, level, auth
context) and emits with empty identity. The phase5 gate cross-check
already only fires when both marker and manifest carry api_name/run_id,
so dropping the required-identity assertion in validatePhase5PassMarker
keeps stale-marker protection intact while accepting markers written
prior to promote.
Closes #963
* fix(cli): tighten phase5 marker cross-check and fall back to runstate identity
Greptile flagged that dropping the unconditional identity requirement
let a zero-identity marker pass every subsequent promote regardless of
manifest run_id rotation. Restore stale-marker protection by moving the
check into validatePhase5Marker, conditioned on the manifest carrying
identity: when manifest.APIName/RunID are present, the marker must
match (and empty marker identity is rejected); when the manifest is
unidentified, empty marker identity is allowed.
To keep the bug fix working for the pre-promote scenario from #963,
writeLiveDogfoodAcceptance now consults runstate via
FindStateByWorkingDir when the manifest hasn't been written yet, so the
marker carries the same api_name/run_id lock promote will later
validate against.
Refs #963
Files touched
M internal/pipeline/live_dogfood.goM internal/pipeline/live_dogfood_test.goM internal/pipeline/phase5_gate.goM internal/pipeline/phase5_gate_test.go
Diff
commit ee13fb1611755cc2964a506b03bf614a44d190eb
Author: Trevin Chow <trevin@trevinchow.com>
Date: Tue May 12 02:11:21 2026 -0700
fix(cli): make dogfood acceptance marker manifest read best-effort (#1179)
* fix(cli): make dogfood acceptance marker manifest read best-effort
Phase 5 dogfood writes phase5-acceptance.json before `lock promote`
creates .printing-press.json, so requiring the manifest at acceptance
time errored out every successful generation. Make the read tolerate a
missing file: when the manifest exists, identity and auth_type still
flow into the marker; when it does not, the marker carries the dogfood
run's own state (matrix size, pass/fail/skip counts, level, auth
context) and emits with empty identity. The phase5 gate cross-check
already only fires when both marker and manifest carry api_name/run_id,
so dropping the required-identity assertion in validatePhase5PassMarker
keeps stale-marker protection intact while accepting markers written
prior to promote.
Closes #963
* fix(cli): tighten phase5 marker cross-check and fall back to runstate identity
Greptile flagged that dropping the unconditional identity requirement
let a zero-identity marker pass every subsequent promote regardless of
manifest run_id rotation. Restore stale-marker protection by moving the
check into validatePhase5Marker, conditioned on the manifest carrying
identity: when manifest.APIName/RunID are present, the marker must
match (and empty marker identity is rejected); when the manifest is
unidentified, empty marker identity is allowed.
To keep the bug fix working for the pre-promote scenario from #963,
writeLiveDogfoodAcceptance now consults runstate via
FindStateByWorkingDir when the manifest hasn't been written yet, so the
marker carries the same api_name/run_id lock promote will later
validate against.
Refs #963
---
internal/pipeline/live_dogfood.go | 52 ++++++++++++++++++------
internal/pipeline/live_dogfood_test.go | 73 +++++++++++++++++++++++++++++++---
internal/pipeline/phase5_gate.go | 39 +++++++++++++-----
internal/pipeline/phase5_gate_test.go | 50 ++++++++++++++++++++++-
4 files changed, 185 insertions(+), 29 deletions(-)
diff --git a/internal/pipeline/live_dogfood.go b/internal/pipeline/live_dogfood.go
index b43108a1..c2e9ebde 100644
--- a/internal/pipeline/live_dogfood.go
+++ b/internal/pipeline/live_dogfood.go
@@ -1101,25 +1101,24 @@ func finalizeLiveDogfoodReport(report *LiveDogfoodReport) {
}
func writeLiveDogfoodAcceptance(opts LiveDogfoodOptions, report *LiveDogfoodReport) error {
- manifest, err := ReadCLIManifest(opts.CLIDir)
- if err != nil {
- return fmt.Errorf("reading CLI manifest for phase5 acceptance: %w", err)
- }
- if manifest.APIName == "" {
- return fmt.Errorf("CLI manifest missing api_name; cannot write phase5 acceptance")
- }
- if manifest.RunID == "" {
- return fmt.Errorf("CLI manifest missing run_id; cannot write phase5 acceptance")
- }
- authType := manifest.AuthType
+ // Identity (api_name/run_id) is recorded so `lock promote`'s cross-check
+ // in validatePhase5Marker can reject stale markers. Three sources, in
+ // order: the working-dir manifest (most authoritative — already merged
+ // catalog/spec data), the runstate for this working dir (covers the
+ // pre-promote case where generate has not written the manifest yet), and
+ // finally an empty fall-back so dogfood still emits a marker for foreign
+ // working dirs. The marker carries empty identity only when neither
+ // source exists, which is the scenario where a downstream gate has no
+ // manifest identity to compare against either.
+ apiName, runID, authType := resolveLiveDogfoodAcceptanceIdentity(opts.CLIDir)
if authType == "" {
authType = "none"
}
marker := Phase5GateMarker{
SchemaVersion: 1,
- APIName: manifest.APIName,
- RunID: manifest.RunID,
+ APIName: apiName,
+ RunID: runID,
Status: "pass",
Level: report.Level,
MatrixSize: report.MatrixSize,
@@ -1144,6 +1143,33 @@ func writeLiveDogfoodAcceptance(opts LiveDogfoodOptions, report *LiveDogfoodRepo
return nil
}
+// 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
+// from issue #963). I/O errors other than "not found" propagate as empty
+// values rather than failing the write — emitting an incomplete marker
+// beats blocking dogfood, and the gate cross-check catches identity drift
+// on the way to promote.
+func resolveLiveDogfoodAcceptanceIdentity(cliDir string) (apiName, runID, authType string) {
+ if manifest, err := ReadCLIManifest(cliDir); err == nil {
+ apiName = manifest.APIName
+ runID = manifest.RunID
+ authType = manifest.AuthType
+ }
+ if apiName != "" && runID != "" {
+ return apiName, runID, authType
+ }
+ if state, err := FindStateByWorkingDir(cliDir); err == nil {
+ if apiName == "" {
+ apiName = state.APIName
+ }
+ if runID == "" {
+ runID = state.RunID
+ }
+ }
+ return apiName, runID, authType
+}
+
func liveDogfoodQuickCommands(commands []liveDogfoodCommand) []liveDogfoodCommand {
if len(commands) <= 2 {
return commands
diff --git a/internal/pipeline/live_dogfood_test.go b/internal/pipeline/live_dogfood_test.go
index dee6b69e..66ea6632 100644
--- a/internal/pipeline/live_dogfood_test.go
+++ b/internal/pipeline/live_dogfood_test.go
@@ -246,23 +246,86 @@ func TestRunLiveDogfoodExplicitBinaryNameMustExist(t *testing.T) {
assert.Contains(t, err.Error(), "missing-pp-cli")
}
-func TestRunLiveDogfoodAcceptanceRequiresManifestIdentity(t *testing.T) {
+func TestRunLiveDogfoodAcceptanceWithoutManifestEmitsMarker(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("test uses a shell script as the fake binary; skip on Windows")
}
+ // Phase 5 dogfood runs before `lock promote` writes the manifest. With
+ // no .printing-press.json on disk and no runstate matching this temp
+ // fixture, --write-acceptance must still emit a marker carrying the
+ // dogfood run's own state. Identity stays empty; the gate cross-check
+ // in validatePhase5Marker only enforces identity when the manifest
+ // supplies it.
dir, binaryName := writeLiveDogfoodFixture(t, true)
require.NoError(t, os.Remove(filepath.Join(dir, CLIManifestFilename)))
- _, err := RunLiveDogfood(LiveDogfoodOptions{
+ markerPath := filepath.Join(t.TempDir(), Phase5AcceptanceFilename)
+ report, err := RunLiveDogfood(LiveDogfoodOptions{
CLIDir: dir,
BinaryName: binaryName,
Level: "full",
Timeout: 2 * time.Second,
- WriteAcceptancePath: filepath.Join(t.TempDir(), Phase5AcceptanceFilename),
+ WriteAcceptancePath: markerPath,
})
- require.Error(t, err)
- assert.Contains(t, err.Error(), "CLI manifest")
+ require.NoError(t, err)
+ require.Equal(t, "PASS", report.Verdict, report.Tests)
+
+ data, err := os.ReadFile(markerPath)
+ require.NoError(t, err)
+ var marker Phase5GateMarker
+ require.NoError(t, json.Unmarshal(data, &marker))
+ assert.Equal(t, "pass", marker.Status)
+ assert.Equal(t, "full", marker.Level)
+ assert.Equal(t, report.MatrixSize, marker.MatrixSize)
+ assert.Equal(t, report.Passed, marker.TestsPassed)
+ assert.Empty(t, marker.APIName, "marker should not invent identity when neither manifest nor runstate supplies it")
+ assert.Empty(t, marker.RunID, "marker should not invent identity when neither manifest nor runstate supplies it")
+ assert.Equal(t, "none", marker.AuthContext.Type)
+
+ // Validation passes against an unidentified manifest because the
+ // cross-check has nothing to enforce.
+ validation := ValidatePhase5Gate(filepath.Dir(markerPath), CLIManifest{AuthType: "none"})
+ assert.True(t, validation.Passed, validation.Detail)
+}
+
+func TestRunLiveDogfoodAcceptanceFallsBackToRunstateIdentity(t *testing.T) {
+ if runtime.GOOS == "windows" {
+ t.Skip("test uses a shell script as the fake binary; skip on Windows")
+ }
+
+ // Pre-promote scenario from issue #963: working dir has no manifest but
+ // runstate identifies the CLI. The marker must record state's
+ // api_name/run_id so the gate cross-check at promote time matches the
+ // manifest lock promote will write.
+ setPressTestEnv(t)
+
+ dir, binaryName := writeLiveDogfoodFixture(t, true)
+ require.NoError(t, os.Remove(filepath.Join(dir, CLIManifestFilename)))
+
+ state := NewState("fixture", dir)
+ require.NoError(t, state.Save())
+
+ markerPath := filepath.Join(t.TempDir(), Phase5AcceptanceFilename)
+ report, err := RunLiveDogfood(LiveDogfoodOptions{
+ CLIDir: dir,
+ BinaryName: binaryName,
+ Level: "full",
+ Timeout: 2 * time.Second,
+ WriteAcceptancePath: markerPath,
+ })
+ require.NoError(t, err)
+ require.Equal(t, "PASS", report.Verdict, report.Tests)
+
+ data, err := os.ReadFile(markerPath)
+ require.NoError(t, err)
+ var marker Phase5GateMarker
+ require.NoError(t, json.Unmarshal(data, &marker))
+ assert.Equal(t, "fixture", marker.APIName, "marker should record runstate api_name when manifest is absent")
+ assert.Equal(t, state.RunID, marker.RunID, "marker should record runstate run_id when manifest is absent")
+
+ validation := ValidatePhase5Gate(filepath.Dir(markerPath), CLIManifest{APIName: "fixture", RunID: state.RunID, AuthType: "none"})
+ assert.True(t, validation.Passed, validation.Detail)
}
// TestFinalizeLiveDogfoodReportVerdictGate exercises the quick-level verdict
diff --git a/internal/pipeline/phase5_gate.go b/internal/pipeline/phase5_gate.go
index fd139fa8..84fe0ac7 100644
--- a/internal/pipeline/phase5_gate.go
+++ b/internal/pipeline/phase5_gate.go
@@ -92,13 +92,31 @@ func validatePhase5Marker(marker Phase5GateMarker, manifest CLIManifest, skipFil
result.Detail = fmt.Sprintf("unsupported phase5 marker schema_version %d", marker.SchemaVersion)
return result
}
- if marker.APIName != "" && manifest.APIName != "" && marker.APIName != manifest.APIName {
- result.Detail = fmt.Sprintf("phase5 marker api_name %q does not match manifest api_name %q", marker.APIName, manifest.APIName)
- return result
+ // Stale-marker protection: when the manifest carries identity, the
+ // marker must carry the same identity. An empty marker.APIName/RunID is
+ // only acceptable when the manifest is itself unidentified (e.g., a
+ // minimal state with no api_name) — otherwise an empty-identity marker
+ // would silently pass every subsequent promote regardless of run_id
+ // rotation.
+ if manifest.APIName != "" {
+ if marker.APIName == "" {
+ result.Detail = "phase5 marker missing api_name (manifest identifies the CLI)"
+ return result
+ }
+ if marker.APIName != manifest.APIName {
+ result.Detail = fmt.Sprintf("phase5 marker api_name %q does not match manifest api_name %q", marker.APIName, manifest.APIName)
+ return result
+ }
}
- if marker.RunID != "" && manifest.RunID != "" && marker.RunID != manifest.RunID {
- result.Detail = fmt.Sprintf("phase5 marker run_id %q does not match manifest run_id %q", marker.RunID, manifest.RunID)
- return result
+ if manifest.RunID != "" {
+ if marker.RunID == "" {
+ result.Detail = "phase5 marker missing run_id (manifest identifies the run)"
+ return result
+ }
+ if marker.RunID != manifest.RunID {
+ result.Detail = fmt.Sprintf("phase5 marker run_id %q does not match manifest run_id %q", marker.RunID, manifest.RunID)
+ return result
+ }
}
switch status {
@@ -142,11 +160,12 @@ func validatePhase5Marker(marker Phase5GateMarker, manifest CLIManifest, skipFil
}
func validatePhase5PassMarker(marker Phase5GateMarker) string {
+ // api_name and run_id are identity tags: the cross-check in
+ // validatePhase5Marker enforces consistency when both marker and
+ // manifest carry them, so requiring them here would reject markers
+ // written before the manifest exists (e.g., dogfood --write-acceptance
+ // run prior to `lock promote`).
switch {
- case strings.TrimSpace(marker.APIName) == "":
- return "phase5 acceptance marker missing api_name"
- case strings.TrimSpace(marker.RunID) == "":
- return "phase5 acceptance marker missing run_id"
case phase5Level(marker) == "":
return "phase5 acceptance marker missing level"
case marker.MatrixSize <= 0:
diff --git a/internal/pipeline/phase5_gate_test.go b/internal/pipeline/phase5_gate_test.go
index c04b5266..f7c59123 100644
--- a/internal/pipeline/phase5_gate_test.go
+++ b/internal/pipeline/phase5_gate_test.go
@@ -357,7 +357,11 @@ func TestValidatePhase5Gate_SkipCannotOverrideManifestAuthType(t *testing.T) {
assert.Contains(t, result.Detail, "does not match")
}
-func TestValidatePhase5Gate_PassMarkerRequiresIdentityAndTestCount(t *testing.T) {
+func TestValidatePhase5Gate_PassMarkerRejectsEmptyIdentityWhenManifestIdentifies(t *testing.T) {
+ // Stale-marker protection: when the manifest identifies the CLI, an
+ // empty-identity marker would otherwise pass every future promote.
+ // Reject it so cross-check enforcement degrades only for the actual
+ // unidentified-manifest case.
proofsDir := t.TempDir()
manifest := CLIManifest{APIName: "test", CLIName: "test-pp-cli", RunID: "run-1", AuthType: "none"}
writePhase5GateMarker(t, proofsDir, Phase5AcceptanceFilename, Phase5GateMarker{
@@ -374,6 +378,50 @@ func TestValidatePhase5Gate_PassMarkerRequiresIdentityAndTestCount(t *testing.T)
assert.Contains(t, result.Detail, "api_name")
}
+func TestValidatePhase5Gate_PassMarkerAllowsEmptyIdentityWhenManifestUnidentified(t *testing.T) {
+ // dogfood --write-acceptance may run for a foreign working dir with no
+ // manifest and no runstate; the marker then has no identity to record
+ // and the gate has no manifest identity to compare against either.
+ // The marker still validates because the cross-check has nothing to
+ // enforce.
+ proofsDir := t.TempDir()
+ manifest := CLIManifest{AuthType: "none"}
+ writePhase5GateMarker(t, proofsDir, Phase5AcceptanceFilename, Phase5GateMarker{
+ SchemaVersion: 1,
+ Status: "pass",
+ Level: "full",
+ MatrixSize: 1,
+ TestsPassed: 1,
+ AuthContext: Phase5AuthContext{Type: "none"},
+ })
+
+ result := ValidatePhase5Gate(proofsDir, manifest)
+ require.True(t, result.Passed, result.Detail)
+ assert.Equal(t, "pass", result.Status)
+}
+
+func TestValidatePhase5Gate_PassMarkerCrossChecksIdentityWhenPresent(t *testing.T) {
+ // When the marker does carry identity, mismatches against the manifest
+ // must still be rejected — this is what prevents stale markers from a
+ // prior run leaking into a fresh promote.
+ proofsDir := t.TempDir()
+ manifest := CLIManifest{APIName: "stripe", CLIName: "stripe-pp-cli", RunID: "run-1", AuthType: "none"}
+ writePhase5GateMarker(t, proofsDir, Phase5AcceptanceFilename, Phase5GateMarker{
+ SchemaVersion: 1,
+ APIName: "notion",
+ RunID: "run-1",
+ Status: "pass",
+ Level: "full",
+ MatrixSize: 1,
+ TestsPassed: 1,
+ AuthContext: Phase5AuthContext{Type: "none"},
+ })
+
+ result := ValidatePhase5Gate(proofsDir, manifest)
+ require.False(t, result.Passed)
+ assert.Contains(t, result.Detail, "does not match")
+}
+
func TestValidatePhase5Gate_SkipMarkerRequiresIdentity(t *testing.T) {
proofsDir := t.TempDir()
manifest := CLIManifest{APIName: "test", CLIName: "test-pp-cli", RunID: "run-1", AuthType: "api_key"}
← a3c3c653 fix(cli): redact $HOME paths from publish-tree JSON artifact
·
back to Cli Printing Press
·
docs(skills): tighten Phase 3 Completion Gate to per-row Cob d0c4caca →