← back to Cli Printing Press
fix(cli): recover RunID in NewMinimalState so lock promote enriches novel_features (#340)
b25c2b9019cb48a39b60e7da84c605ad0b3e9166 · 2026-04-27 02:35:51 -0700 · Trevin Chow
`printing-press lock promote` was silently producing manifests without
`novel_features`, then `publish validate` failed the transcendence check.
The retro initially diagnosed this as `lock promote` "bypassing the
enrichment path" and proposed extracting a helper. ce-doc-review's
feasibility pass corrected the diagnosis: lock.go:241 calls
PromoteWorkingCLI which calls writeCLIManifestForPublish — the SAME
enriching path the publish-pipeline uses. Refs issue #336 (U3).
The actual bug is upstream of the enrichment block. When
FindStateByWorkingDir(dir) fails (the working dir doesn't match any
current runstate), lock.go falls back to NewMinimalState(cliName, dir).
NewMinimalState set APIName and WorkingDir but not RunID — so
state.PipelineDir() returned RunPipelineDir("") which produces a path
with an empty segment under ~/printing-press/.runstate/<scope>/runs/.
LoadResearch then ENOENT'd silently, and enrichment fell through.
The fix has two parts:
1. NewMinimalState recovers RunID from the scoped runstate registry.
After setting APIName and WorkingDir, NewMinimalState now calls
LoadState(apiName) which routes through resolveStatePath →
findRunstateStatePath → the scoped runstate registry. If a state
file matching this APIName exists (already filtered by the registry's
APIName predicate), borrow its RunID and Scope. The caller's
WorkingDir/OutputDir win over the recovered state's so promotion
still copies from the right source.
On any error — no registry hit, parse failure, IO error — fall
through to today's empty-RunID minimal state. Plan-driven CLIs that
genuinely have no prior runstate keep working.
2. publish.go gains a debug-level breadcrumb when LoadResearch ENOENTs.
Previously the enrichment block silently skipped on `LoadResearch`
error, which is exactly what made U3's bug invisible for so long.
The breadcrumb names the path that was attempted and the state's
current RunID, so future regressions of either the recovery path
or the research-write path leave a visible signal in stderr without
becoming a user-facing error.
Tests in internal/pipeline/minimal_state_test.go cover:
- back-compat: no runstate → empty RunID (today's behavior preserved)
- happy path: registry hit → RunID + Scope borrowed; caller's WorkingDir
wins over the loaded state's
- PipelineDir resolves to a real path containing the RunID after
recovery (the U3 contract)
- APIName mismatch: a runstate for a different API does not get
adopted (defends against weakened registry filtering)
- Scope is preserved alongside RunID
Verified via existing TestPromoteWorkingCLI_MinimalStateNoRunstate that
the promote flow continues to work when no runstate exists.
Refs #336, #333
Files touched
A internal/pipeline/minimal_state_test.goM internal/pipeline/publish.goM internal/pipeline/state.go
Diff
commit b25c2b9019cb48a39b60e7da84c605ad0b3e9166
Author: Trevin Chow <trevin@trevinchow.com>
Date: Mon Apr 27 02:35:51 2026 -0700
fix(cli): recover RunID in NewMinimalState so lock promote enriches novel_features (#340)
`printing-press lock promote` was silently producing manifests without
`novel_features`, then `publish validate` failed the transcendence check.
The retro initially diagnosed this as `lock promote` "bypassing the
enrichment path" and proposed extracting a helper. ce-doc-review's
feasibility pass corrected the diagnosis: lock.go:241 calls
PromoteWorkingCLI which calls writeCLIManifestForPublish — the SAME
enriching path the publish-pipeline uses. Refs issue #336 (U3).
The actual bug is upstream of the enrichment block. When
FindStateByWorkingDir(dir) fails (the working dir doesn't match any
current runstate), lock.go falls back to NewMinimalState(cliName, dir).
NewMinimalState set APIName and WorkingDir but not RunID — so
state.PipelineDir() returned RunPipelineDir("") which produces a path
with an empty segment under ~/printing-press/.runstate/<scope>/runs/.
LoadResearch then ENOENT'd silently, and enrichment fell through.
The fix has two parts:
1. NewMinimalState recovers RunID from the scoped runstate registry.
After setting APIName and WorkingDir, NewMinimalState now calls
LoadState(apiName) which routes through resolveStatePath →
findRunstateStatePath → the scoped runstate registry. If a state
file matching this APIName exists (already filtered by the registry's
APIName predicate), borrow its RunID and Scope. The caller's
WorkingDir/OutputDir win over the recovered state's so promotion
still copies from the right source.
On any error — no registry hit, parse failure, IO error — fall
through to today's empty-RunID minimal state. Plan-driven CLIs that
genuinely have no prior runstate keep working.
2. publish.go gains a debug-level breadcrumb when LoadResearch ENOENTs.
Previously the enrichment block silently skipped on `LoadResearch`
error, which is exactly what made U3's bug invisible for so long.
The breadcrumb names the path that was attempted and the state's
current RunID, so future regressions of either the recovery path
or the research-write path leave a visible signal in stderr without
becoming a user-facing error.
Tests in internal/pipeline/minimal_state_test.go cover:
- back-compat: no runstate → empty RunID (today's behavior preserved)
- happy path: registry hit → RunID + Scope borrowed; caller's WorkingDir
wins over the loaded state's
- PipelineDir resolves to a real path containing the RunID after
recovery (the U3 contract)
- APIName mismatch: a runstate for a different API does not get
adopted (defends against weakened registry filtering)
- Scope is preserved alongside RunID
Verified via existing TestPromoteWorkingCLI_MinimalStateNoRunstate that
the promote flow continues to work when no runstate exists.
Refs #336, #333
---
internal/pipeline/minimal_state_test.go | 106 ++++++++++++++++++++++++++++++++
internal/pipeline/publish.go | 15 ++++-
internal/pipeline/state.go | 38 ++++++++++--
3 files changed, 153 insertions(+), 6 deletions(-)
diff --git a/internal/pipeline/minimal_state_test.go b/internal/pipeline/minimal_state_test.go
new file mode 100644
index 00000000..88f2103e
--- /dev/null
+++ b/internal/pipeline/minimal_state_test.go
@@ -0,0 +1,106 @@
+// Copyright 2026 trevin-chow. Licensed under Apache-2.0. See LICENSE.
+
+package pipeline
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// TestNewMinimalState_NoRunstateLeavesRunIDEmpty exercises the back-compat
+// path: a working dir with no adjacent runstate must produce a minimal state
+// with RunID empty (today's behavior). Hand-built CLIs that genuinely have
+// no prior runstate must keep working.
+func TestNewMinimalState_NoRunstateLeavesRunIDEmpty(t *testing.T) {
+ setPressTestEnv(t)
+
+ state := NewMinimalState("hand-built-pp-cli", "/tmp/never-existed")
+
+ assert.Equal(t, "hand-built", state.APIName, "APIName must be the trimmed CLI name")
+ assert.Equal(t, "/tmp/never-existed", state.WorkingDir)
+ assert.Equal(t, "/tmp/never-existed", state.OutputDir)
+ assert.Empty(t, state.RunID, "RunID must stay empty when no runstate exists")
+}
+
+// TestNewMinimalState_RecoversRunIDFromRegistry exercises the U3 fix: when
+// a runstate exists for this API in the scoped runstate registry, its RunID
+// is borrowed so downstream state.PipelineDir() resolves to a path where
+// research.json can be found. The caller's WorkingDir wins over the loaded
+// state's so promotion still copies from the right source.
+func TestNewMinimalState_RecoversRunIDFromRegistry(t *testing.T) {
+ setPressTestEnv(t)
+
+ // Plant a runstate as if a prior `generate` had stored one.
+ prior := NewState("recovery-test", "/tmp/prior-working-dir")
+ require.NotEmpty(t, prior.RunID, "NewState should mint a RunID")
+ require.NoError(t, prior.Save(), "saving the prior runstate must succeed")
+
+ // Now invoke NewMinimalState with a DIFFERENT working dir for the
+ // same API (e.g., user's `lock promote` from a fresh checkout).
+ state := NewMinimalState("recovery-test-pp-cli", "/tmp/different-working-dir")
+
+ assert.Equal(t, "recovery-test", state.APIName)
+ assert.Equal(t, "/tmp/different-working-dir", state.WorkingDir,
+ "caller's WorkingDir must win — promotion copies from this dir")
+ assert.Equal(t, "/tmp/different-working-dir", state.OutputDir,
+ "caller's OutputDir must win")
+ assert.Equal(t, prior.RunID, state.RunID,
+ "RunID must be borrowed from the recovered runstate so PipelineDir resolves correctly")
+}
+
+// TestNewMinimalState_PipelineDirResolvesAfterRecovery proves the U3 contract:
+// after recovery, state.PipelineDir() returns a real path where research.json
+// would live, not the bogus RunPipelineDir("") that the pre-fix behavior
+// produced.
+func TestNewMinimalState_PipelineDirResolvesAfterRecovery(t *testing.T) {
+ setPressTestEnv(t)
+
+ prior := NewState("pipeline-dir-test", "/tmp/working")
+ require.NoError(t, prior.Save())
+
+ state := NewMinimalState("pipeline-dir-test-pp-cli", "/tmp/different-working")
+ pipelineDir := state.PipelineDir()
+
+ // The pre-fix path was RunPipelineDir("") which contains an empty
+ // segment. Post-fix, the path must contain the recovered RunID.
+ assert.Contains(t, pipelineDir, prior.RunID,
+ "PipelineDir must include the recovered RunID, not be empty")
+ assert.NotContains(t, pipelineDir, "//runs/pipeline",
+ "PipelineDir must not contain the empty-RunID artifact `//runs/pipeline`")
+}
+
+// TestNewMinimalState_APINameMismatchDoesNotAdoptStaleRunID confirms that
+// a runstate for a DIFFERENT API name does not get adopted. findRunstateStatePath
+// already filters by APIName, so this should be a non-event — but the test
+// guards against future regressions if the filtering is ever weakened.
+func TestNewMinimalState_APINameMismatchDoesNotAdoptStaleRunID(t *testing.T) {
+ setPressTestEnv(t)
+
+ // Plant runstate for a different API.
+ other := NewState("some-other-api", "/tmp/other-working")
+ require.NoError(t, other.Save())
+
+ // Create minimal state for an API with no prior runstate.
+ state := NewMinimalState("brand-new-pp-cli", "/tmp/brand-new-working")
+
+ assert.Equal(t, "brand-new", state.APIName)
+ assert.Empty(t, state.RunID,
+ "RunID must NOT be borrowed from a runstate with a different APIName")
+}
+
+// TestNewMinimalState_PreservesScopeFromRecoveredState confirms Scope is
+// copied alongside RunID. Without Scope, downstream registry lookups (e.g.,
+// findRunstateStatePath called transitively) may fail with empty-scope
+// path resolution.
+func TestNewMinimalState_PreservesScopeFromRecoveredState(t *testing.T) {
+ setPressTestEnv(t)
+
+ prior := NewState("scope-test", "/tmp/working")
+ require.NoError(t, prior.Save())
+ require.Equal(t, "test-scope", prior.Scope, "test env sets test-scope")
+
+ state := NewMinimalState("scope-test-pp-cli", "/tmp/different")
+ assert.Equal(t, "test-scope", state.Scope, "Scope must be borrowed from the recovered state")
+}
diff --git a/internal/pipeline/publish.go b/internal/pipeline/publish.go
index 76964c27..349a9cff 100644
--- a/internal/pipeline/publish.go
+++ b/internal/pipeline/publish.go
@@ -262,8 +262,14 @@ func writeCLIManifestForPublish(state *PipelineState, dir string) error {
m.Description = entry.Description
}
- // Load novel features from research.json if available.
- if research, err := LoadResearch(state.PipelineDir()); err == nil && research.NovelFeaturesBuilt != nil {
+ // Load novel features from research.json if available. When research.json
+ // can't be located (typically because state.RunID is empty in a fallback
+ // scenario, or the run's pipeline dir was GCed), emit a debug breadcrumb
+ // to stderr so the silent dropout from earlier versions is no longer
+ // silent. The breadcrumb names the path that was attempted; the caller
+ // (lock promote, publish) shows it but does not treat it as an error.
+ pipelineDir := state.PipelineDir()
+ if research, err := LoadResearch(pipelineDir); err == nil && research.NovelFeaturesBuilt != nil {
for _, nf := range *research.NovelFeaturesBuilt {
m.NovelFeatures = append(m.NovelFeatures, NovelFeatureManifest{
Name: nf.Name,
@@ -271,6 +277,11 @@ func writeCLIManifestForPublish(state *PipelineState, dir string) error {
Description: nf.Description,
})
}
+ } else {
+ fmt.Fprintf(os.Stderr,
+ "debug: research.json not found at %s; skipping novel_features enrichment "+
+ "(state.RunID=%q)\n",
+ filepath.Join(pipelineDir, "research.json"), state.RunID)
}
return WriteCLIManifest(dir, m)
diff --git a/internal/pipeline/state.go b/internal/pipeline/state.go
index 3ca4a51f..65e7f4e3 100644
--- a/internal/pipeline/state.go
+++ b/internal/pipeline/state.go
@@ -303,17 +303,47 @@ func FindStateByWorkingDir(dir string) (*PipelineState, error) {
}
// NewMinimalState creates a lightweight state for CLIs that skipped the
-// generate pipeline (e.g. plan-driven CLIs). It carries enough metadata
-// for promote to copy the directory and write a manifest.
+// generate pipeline (e.g. plan-driven CLIs) or whose working directory
+// doesn't match any current runstate via FindStateByWorkingDir.
+//
+// If a runstate exists for this API name in the scoped runstate registry,
+// its RunID and Scope are borrowed so downstream consumers (notably
+// writeCLIManifestForPublish's research.json enrichment via
+// state.PipelineDir()) can locate the run's pipeline artifacts. The
+// caller's WorkingDir/OutputDir win over the recovered state's so
+// promotion still copies from the correct source. APIName matching is
+// enforced by findRunstateStatePath inside resolveStatePath / LoadState;
+// this function does not re-validate.
+//
+// When no recoverable runstate exists, RunID stays empty and downstream
+// research.json lookup falls through to its no-op path. The caller can
+// observe the recovery decision via state.RunID after construction.
func NewMinimalState(cliName, workingDir string) *PipelineState {
- return &PipelineState{
+ apiName := naming.TrimCLISuffix(cliName)
+ state := &PipelineState{
Version: currentStateVersion,
- APIName: naming.TrimCLISuffix(cliName),
+ APIName: apiName,
WorkingDir: workingDir,
OutputDir: workingDir,
StartedAt: time.Now(),
Phases: make(map[string]PhaseState),
}
+
+ // Recover RunID from the scoped runstate registry when possible. A
+ // matching state file means a prior generation for this API stored
+ // its RunID and pipeline artifacts on disk; downstream enrichment
+ // can find them via state.PipelineDir(). On any error (no registry
+ // hit, parse failure, IO error) fall through to the empty-RunID
+ // minimal state — preserving today's behavior for plan-driven CLIs
+ // that genuinely have no prior runstate.
+ if loaded, err := LoadState(apiName); err == nil && loaded != nil && loaded.RunID != "" {
+ state.RunID = loaded.RunID
+ if state.Scope == "" {
+ state.Scope = loaded.Scope
+ }
+ }
+
+ return state
}
// NewState creates a fresh pipeline state.
← 907c6986 feat(cli): add unknown-command check + sync test for verify-
·
back to Cli Printing Press
·
fix(cli): Cal.com retro #334 — 4 of 5 P1 machine fixes (per- c74dcd45 →