← back to Cli Printing Press
fix(cli): guard nil Phases in LoadState to prevent lock promote panic (#1193)
28e2fa1b44740135699e1bf4502411ae9e363db6 · 2026-05-12 14:32:15 -0700 · Trevin Chow
* fix(cli): guard nil Phases in LoadState to prevent lock promote panic
When state.json on disk contains "phases": null, the version-migration
loop in LoadState assigned into the unmarshaled nil map and panicked
with "assignment to entry in nil map". This blocked any second run of
`lock promote` for the same CLI, because the first promote left a state
file in this shape and FindStateByWorkingDir then fell back to
NewMinimalState -> LoadState.
Initialize Phases to a non-nil map before the migration loop runs, and
defensively initialize it in save() so we never re-emit `phases: null`.
Closes #1189
* docs(cli): note Phases nil-guard in PIPELINE.md
AGENTS.md requires updating PIPELINE.md when internal/pipeline/state.go
changes. Confirm phase names, ordering, transitions, and migration
semantics are unaffected by the defensive nil-guard added to
LoadState/save().
Refs #1189
Files touched
M docs/PIPELINE.mdM internal/pipeline/state.goM internal/pipeline/state_test.go
Diff
commit 28e2fa1b44740135699e1bf4502411ae9e363db6
Author: Trevin Chow <trevin@trevinchow.com>
Date: Tue May 12 14:32:15 2026 -0700
fix(cli): guard nil Phases in LoadState to prevent lock promote panic (#1193)
* fix(cli): guard nil Phases in LoadState to prevent lock promote panic
When state.json on disk contains "phases": null, the version-migration
loop in LoadState assigned into the unmarshaled nil map and panicked
with "assignment to entry in nil map". This blocked any second run of
`lock promote` for the same CLI, because the first promote left a state
file in this shape and FindStateByWorkingDir then fell back to
NewMinimalState -> LoadState.
Initialize Phases to a non-nil map before the migration loop runs, and
defensively initialize it in save() so we never re-emit `phases: null`.
Closes #1189
* docs(cli): note Phases nil-guard in PIPELINE.md
AGENTS.md requires updating PIPELINE.md when internal/pipeline/state.go
changes. Confirm phase names, ordering, transitions, and migration
semantics are unaffected by the defensive nil-guard added to
LoadState/save().
Refs #1189
---
docs/PIPELINE.md | 2 ++
internal/pipeline/state.go | 8 +++++++
internal/pipeline/state_test.go | 52 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 62 insertions(+)
diff --git a/docs/PIPELINE.md b/docs/PIPELINE.md
index b478ae3f..e96e8813 100644
--- a/docs/PIPELINE.md
+++ b/docs/PIPELINE.md
@@ -39,6 +39,8 @@ Every phase has two orthogonal status fields:
`Status` tracks the execution state of the phase. `PlanStatus` tracks the state of the phase's plan file: a freshly-initialized phase has a `seed` plan; once expanded by the planner it becomes `expanded`; once the phase finishes it becomes `completed`. Both are defined in `internal/pipeline/state.go`.
+`LoadState` and `save()` defensively initialize the on-disk `phases` map to an empty object when it is unmarshaled as nil. Phase names, ordering, transitions, and the version-migration semantics described in this document are unaffected by that guard.
+
## Phases
Each phase below lists four fields: what it consumes, what it produces, what gates it must pass, and which artifacts it leaves on disk.
diff --git a/internal/pipeline/state.go b/internal/pipeline/state.go
index 4b31f54c..634563e8 100644
--- a/internal/pipeline/state.go
+++ b/internal/pipeline/state.go
@@ -416,6 +416,9 @@ func NewStateWithRun(apiName, outputDir, runID, scope string) *PipelineState {
}
func (s *PipelineState) save(updateCurrentPointer bool) error {
+ if s.Phases == nil {
+ s.Phases = make(map[string]PhaseState)
+ }
if s.Scope == "" {
s.Scope = WorkspaceScope()
}
@@ -480,6 +483,11 @@ func LoadState(apiName string) (*PipelineState, error) {
}
needsSave := false
+ // Must run before the version-migration loop below, which assigns into s.Phases.
+ if s.Phases == nil {
+ s.Phases = make(map[string]PhaseState)
+ needsSave = true
+ }
if s.RunID == "" {
runID, err := newRunID(time.Now())
if err != nil {
diff --git a/internal/pipeline/state_test.go b/internal/pipeline/state_test.go
index d470f8c4..0cd0765a 100644
--- a/internal/pipeline/state_test.go
+++ b/internal/pipeline/state_test.go
@@ -2,6 +2,7 @@ package pipeline
import (
"encoding/json"
+ "fmt"
"os"
"path/filepath"
"testing"
@@ -292,3 +293,54 @@ func TestResolveStatePathPrefersLegacyOverExcludedRunstateScratch(t *testing.T)
assert.Equal(t, "/tmp/legacy-cli", loaded.OutputDir)
assert.NotEqual(t, "run-scratch", loaded.RunID)
}
+
+func TestLoadStateHandlesNullPhases(t *testing.T) {
+ cases := []struct {
+ name string
+ version int
+ }{
+ {"old-version triggers migration backfill", 0},
+ {"current-version skips migration loop", currentStateVersion},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ setPressTestEnv(t)
+ apiName := "nil-phases-test"
+ dir := PipelineDir(apiName)
+ require.NoError(t, os.MkdirAll(dir, 0o755))
+
+ raw := fmt.Appendf(nil, `{
+ "version": %d,
+ "api_name": "%s",
+ "output_dir": "/tmp/nil-phases-cli",
+ "working_dir": "/tmp/nil-phases-cli",
+ "phases": null
+}`, tc.version, apiName)
+ require.NoError(t, os.WriteFile(StatePath(apiName), raw, 0o644))
+
+ loaded, err := LoadState(apiName)
+ require.NoError(t, err)
+ require.NotNil(t, loaded.Phases)
+ })
+ }
+}
+
+func TestSavePersistsEmptyPhasesAsObject(t *testing.T) {
+ setPressTestEnv(t)
+ apiName := "save-nil-phases-test"
+
+ state := &PipelineState{
+ Version: currentStateVersion,
+ APIName: apiName,
+ RunID: "run-save-nil",
+ Scope: "test-scope",
+ OutputDir: "/tmp/save-nil-cli",
+ WorkingDir: "/tmp/save-nil-cli",
+ }
+ require.NoError(t, state.Save())
+
+ data, err := os.ReadFile(state.StatePath())
+ require.NoError(t, err)
+ assert.NotContains(t, string(data), `"phases": null`)
+ assert.Contains(t, string(data), `"phases":`)
+}
← 7010c102 fix(ci): fold skill docs into lint gate (#1226)
·
back to Cli Printing Press
·
fix(cli): make .printing-press.json authoritative for parsed bb9b9cb3 →