[object Object]

← back to Cli Printing Press

fix(phase5): explain accepted marker levels (#647)

d4d10a8a5fbf90ed90f9eb60613288d8bd13d7cc · 2026-05-07 02:02:42 -0700 · Dinakar Sarbada

* fix(phase5): explain accepted marker levels

* fix(cli): centralize phase5 acceptance levels

Derive Phase 5 gate diagnostics and live dogfood level validation from the same accepted-level constants so the allow-list cannot drift between the validator and the error message.

---------

Co-authored-by: Dinakar Sarbada <dinakars777@users.noreply.github.com>
Co-authored-by: Trevin Chow <trevin@trevinchow.com>

Files touched

Diff

commit d4d10a8a5fbf90ed90f9eb60613288d8bd13d7cc
Author: Dinakar Sarbada <sarbadadinu@gmail.com>
Date:   Thu May 7 02:02:42 2026 -0700

    fix(phase5): explain accepted marker levels (#647)
    
    * fix(phase5): explain accepted marker levels
    
    * fix(cli): centralize phase5 acceptance levels
    
    Derive Phase 5 gate diagnostics and live dogfood level validation from the same accepted-level constants so the allow-list cannot drift between the validator and the error message.
    
    ---------
    
    Co-authored-by: Dinakar Sarbada <dinakars777@users.noreply.github.com>
    Co-authored-by: Trevin Chow <trevin@trevinchow.com>
---
 internal/pipeline/live_dogfood.go     |  4 ++--
 internal/pipeline/lock_test.go        | 34 ++++++++++++++++++++++++++
 internal/pipeline/phase5_gate.go      | 14 ++++++++---
 internal/pipeline/phase5_gate_test.go | 45 +++++++++++++++++++++++++++++++++++
 4 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/internal/pipeline/live_dogfood.go b/internal/pipeline/live_dogfood.go
index 6fb42136..b43108a1 100644
--- a/internal/pipeline/live_dogfood.go
+++ b/internal/pipeline/live_dogfood.go
@@ -1157,9 +1157,9 @@ func normalizeLiveDogfoodLevel(level string) (string, error) {
 		return "full", nil
 	}
 	switch level {
-	case "quick", "full":
+	case phase5AcceptanceLevelQuick, phase5AcceptanceLevelFull:
 		return level, nil
 	default:
-		return "", fmt.Errorf("invalid live dogfood level %q (expected quick or full)", level)
+		return "", fmt.Errorf("invalid live dogfood level %q (expected %s)", level, strings.Join(phase5AcceptedAcceptanceLevels, " or "))
 	}
 }
diff --git a/internal/pipeline/lock_test.go b/internal/pipeline/lock_test.go
index 5d078378..5eedaf74 100644
--- a/internal/pipeline/lock_test.go
+++ b/internal/pipeline/lock_test.go
@@ -557,6 +557,40 @@ func TestPromoteWorkingCLI_RequiresPhase5GateForRunstatePromote(t *testing.T) {
 	assert.ErrorIs(t, statErr, os.ErrNotExist)
 }
 
+func TestPromoteWorkingCLI_RejectsManualPhase5Marker(t *testing.T) {
+	tmp := t.TempDir()
+	t.Setenv("PRINTING_PRESS_HOME", tmp)
+	t.Setenv("PRINTING_PRESS_SCOPE", "test-scope")
+	t.Setenv("PRINTING_PRESS_REPO_ROOT", tmp)
+
+	workDir := filepath.Join(tmp, "working", "test-pp-cli")
+	require.NoError(t, os.MkdirAll(workDir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(workDir, "go.mod"), []byte("module test-pp-cli\n\ngo 1.21\n"), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(workDir, "main.go"), []byte("package main\nfunc main() {}\n"), 0o644))
+
+	state := NewStateWithRun("test", workDir, "run-manual-phase5", "test-scope")
+	writePhase5GateMarker(t, state.ProofsDir(), Phase5AcceptanceFilename, Phase5GateMarker{
+		SchemaVersion: 1,
+		APIName:       state.APIName,
+		RunID:         state.RunID,
+		Status:        "pass",
+		Level:         "manual",
+		MatrixSize:    1,
+		TestsPassed:   1,
+		TestsFailed:   0,
+		AuthContext:   Phase5AuthContext{Type: "none"},
+	})
+
+	err := PromoteWorkingCLI("test-pp-cli", workDir, state)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "unknown phase5 acceptance level")
+	assert.Contains(t, err.Error(), "quick, full")
+	assert.Contains(t, err.Error(), "dogfood --live --write-acceptance")
+
+	_, statErr := os.Stat(filepath.Join(PublishedLibraryRoot(), "test"))
+	assert.ErrorIs(t, statErr, os.ErrNotExist)
+}
+
 func TestPromoteWorkingCLI_MinimalStateNoRunstate(t *testing.T) {
 	tmp := t.TempDir()
 	t.Setenv("PRINTING_PRESS_HOME", tmp)
diff --git a/internal/pipeline/phase5_gate.go b/internal/pipeline/phase5_gate.go
index e9844fad..fd139fa8 100644
--- a/internal/pipeline/phase5_gate.go
+++ b/internal/pipeline/phase5_gate.go
@@ -11,8 +11,16 @@ import (
 const (
 	Phase5AcceptanceFilename = "phase5-acceptance.json"
 	Phase5SkipFilename       = "phase5-skip.json"
+
+	phase5AcceptanceLevelQuick = "quick"
+	phase5AcceptanceLevelFull  = "full"
 )
 
+var phase5AcceptedAcceptanceLevels = []string{
+	phase5AcceptanceLevelQuick,
+	phase5AcceptanceLevelFull,
+}
+
 type Phase5AuthContext struct {
 	Type                    string `json:"type,omitempty"`
 	APIKeyAvailable         bool   `json:"api_key_available,omitempty"`
@@ -153,7 +161,7 @@ func validatePhase5PassMarker(marker Phase5GateMarker) string {
 func phase5AcceptancePassed(marker Phase5GateMarker) (bool, string) {
 	level := phase5Level(marker)
 	switch level {
-	case "quick":
+	case phase5AcceptanceLevelQuick:
 		if marker.TestsFailed != 0 {
 			return false, fmt.Sprintf("phase5 quick acceptance has %d failed tests", marker.TestsFailed)
 		}
@@ -173,7 +181,7 @@ func phase5AcceptancePassed(marker Phase5GateMarker) (bool, string) {
 			return false, fmt.Sprintf("phase5 quick acceptance requires at least %d/%d tests passed-or-skipped, got %d", threshold, marker.MatrixSize, passOrSkip)
 		}
 		return true, ""
-	case "full":
+	case phase5AcceptanceLevelFull:
 		if marker.TestsFailed != 0 {
 			return false, fmt.Sprintf("phase5 full acceptance has %d failed tests", marker.TestsFailed)
 		}
@@ -182,7 +190,7 @@ func phase5AcceptancePassed(marker Phase5GateMarker) (bool, string) {
 		}
 		return true, ""
 	default:
-		return false, fmt.Sprintf("unknown phase5 acceptance level %q", marker.Level)
+		return false, fmt.Sprintf("unknown phase5 acceptance level %q (accepted: %s; prefer `printing-press dogfood --live --write-acceptance` to generate %s)", marker.Level, strings.Join(phase5AcceptedAcceptanceLevels, ", "), Phase5AcceptanceFilename)
 	}
 }
 
diff --git a/internal/pipeline/phase5_gate_test.go b/internal/pipeline/phase5_gate_test.go
index a7af3c51..c04b5266 100644
--- a/internal/pipeline/phase5_gate_test.go
+++ b/internal/pipeline/phase5_gate_test.go
@@ -240,6 +240,51 @@ func TestValidatePhase5Gate_FullPassRejectsFailures(t *testing.T) {
 	assert.Contains(t, result.Detail, "full")
 }
 
+func TestValidatePhase5Gate_ManualLevelDocumentsAcceptedValues(t *testing.T) {
+	proofsDir := t.TempDir()
+	manifest := CLIManifest{APIName: "test", CLIName: "test-pp-cli", RunID: "run-1", AuthType: "none"}
+	writePhase5GateMarker(t, proofsDir, Phase5AcceptanceFilename, Phase5GateMarker{
+		SchemaVersion: 1,
+		APIName:       "test",
+		RunID:         "run-1",
+		Status:        "pass",
+		Level:         "manual",
+		MatrixSize:    2,
+		TestsPassed:   2,
+		TestsFailed:   0,
+		AuthContext:   Phase5AuthContext{Type: "none"},
+	})
+
+	result := ValidatePhase5Gate(proofsDir, manifest)
+	require.False(t, result.Passed)
+	assert.Contains(t, result.Detail, `"manual"`)
+	assert.Contains(t, result.Detail, "accepted: quick, full")
+	assert.Contains(t, result.Detail, "dogfood --live --write-acceptance")
+}
+
+func TestValidatePhase5Gate_UnknownLevelDocumentsAcceptedValues(t *testing.T) {
+	proofsDir := t.TempDir()
+	manifest := CLIManifest{APIName: "test", CLIName: "test-pp-cli", RunID: "run-1", AuthType: "none"}
+	writePhase5GateMarker(t, proofsDir, Phase5AcceptanceFilename, Phase5GateMarker{
+		SchemaVersion: 1,
+		APIName:       "test",
+		RunID:         "run-1",
+		Status:        "pass",
+		Level:         "smoke",
+		MatrixSize:    1,
+		TestsPassed:   1,
+		TestsFailed:   0,
+		AuthContext:   Phase5AuthContext{Type: "none"},
+	})
+
+	result := ValidatePhase5Gate(proofsDir, manifest)
+	require.False(t, result.Passed)
+	assert.Contains(t, result.Detail, `"smoke"`)
+	assert.Contains(t, result.Detail, "accepted: quick, full")
+	assert.NotContains(t, result.Detail, "manual")
+	assert.Contains(t, result.Detail, "dogfood --live --write-acceptance")
+}
+
 func TestValidatePhase5Gate_NoAuthRequiresPassMarker(t *testing.T) {
 	proofsDir := t.TempDir()
 	manifest := CLIManifest{APIName: "test", CLIName: "test-pp-cli", RunID: "run-1", AuthType: "none"}

← 289b48c5 docs(cli): group install paths at top of generated README (#  ·  back to Cli Printing Press  ·  chore(skills): drop compound-engineering plugin and retro ha a80cb5d4 →