From e66c0b2ab20ff4d9827dd1ff90fcb4a4ff065cab Mon Sep 17 00:00:00 2001
From: Denis Bilenko <denis.bilenko@databricks.com>
Date: Thu, 20 Aug 2026 11:04:20 +0200
Subject: [PATCH] direct: leave state closed when Open fails

Open assigns db.Path before every fallible step, so a failed open left the
receiver half-initialized: the next Open on the same value panicked with
"state already opened" instead of reporting the real error.

Move the body into unlockedOpen and reset the receiver when it fails.
Finalize now shares the same reset helper.

Co-authored-by: Isaac
---
 bundle/direct/dstate/state.go      | 27 ++++++++++++++++++++++++---
 bundle/direct/dstate/state_test.go | 20 ++++++++++++++++++++
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/bundle/direct/dstate/state.go b/bundle/direct/dstate/state.go
index d7c1e8e44e..60b698e49a 100644
--- a/bundle/direct/dstate/state.go
+++ b/bundle/direct/dstate/state.go
@@ -238,6 +238,29 @@ func (db *DeploymentState) Open(ctx context.Context, path string, withRecovery W
 		panic(fmt.Sprintf("state already opened: %v, cannot open %v", db.Path, path))
 	}
 
+	err := db.unlockedOpen(ctx, path, withRecovery, withWrite)
+	if err != nil {
+		// A failed open must leave the receiver closed. unlockedOpen assigns
+		// db.Path before every fallible step, so without this the receiver stays
+		// half-initialized and the next Open on it hits the panic above instead
+		// of reporting the real error.
+		db.reset()
+	}
+	return err
+}
+
+// reset returns the receiver to the not-opened state. Callers must hold db.mu.
+func (db *DeploymentState) reset() {
+	if db.walFile != nil {
+		db.walFile.Close()
+		db.walFile = nil
+	}
+	db.Path = ""
+	db.Data = Database{}
+	db.stateIDs = nil
+}
+
+func (db *DeploymentState) unlockedOpen(ctx context.Context, path string, withRecovery WithRecovery, withWrite WithWrite) error {
 	db.Path = path
 	data, err := os.ReadFile(db.Path)
 	if err != nil {
@@ -463,9 +486,7 @@ func (db *DeploymentState) Finalize(ctx context.Context) (resourcestate.Exported
 
 	state := ExportStateFromData(db.Data)
 
-	db.Path = ""
-	db.Data = Database{}
-	db.stateIDs = nil
+	db.reset()
 
 	return state, err
 }
diff --git a/bundle/direct/dstate/state_test.go b/bundle/direct/dstate/state_test.go
index 3b5dc06221..34ed9ff1c5 100644
--- a/bundle/direct/dstate/state_test.go
+++ b/bundle/direct/dstate/state_test.go
@@ -261,3 +261,23 @@ func TestGetOrInitLineageReadableBeforeWriteAndPersisted(t *testing.T) {
 	assert.Equal(t, lineage, reopened.Data.Lineage)
 	mustFinalize(t, &reopened)
 }
+
+// TestOpenFailureLeavesStateClosed pins that a failed Open leaves the receiver
+// closed. Open assigns db.Path before the steps that can fail, so an unreadable
+// state file used to leave Path set: the next Open on the same value panicked
+// with "state already opened" instead of reporting the real error.
+func TestOpenFailureLeavesStateClosed(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "state.json")
+	require.NoError(t, os.WriteFile(path, []byte("{not json"), 0o600))
+
+	var db DeploymentState
+	require.Error(t, db.Open(t.Context(), path, WithRecovery(true), WithWrite(true)))
+	assert.Empty(t, db.Path)
+
+	// Once the state file is readable, the same receiver opens without panicking.
+	seed := `{"state_version":2,"cli_version":"0.1.2","lineage":"test-lineage","serial":1,"state":{}}`
+	require.NoError(t, os.WriteFile(path, []byte(seed), 0o600))
+	require.NoError(t, db.Open(t.Context(), path, WithRecovery(true), WithWrite(true)))
+	assert.Equal(t, "test-lineage", db.Data.Lineage)
+	mustFinalize(t, &db)
+}