Merge pull request #1714 from entireio/fix/1140-enable-state

fix(setup): enable writes the enabled flag to the resolved settings scope

Author: suhaanthayyil

cmd/entire/cli/integration_test/setup_cmd_test.go

@@ -267,3 +267,93 @@ func TestHooksRunAfterLocalOnlyEnable(t *testing.T) {
 		t.Fatal("commit has no Entire-Checkpoint trailer — hooks silently no-op'd with only settings.local.json")
 	}
 }
+
+// TestEnableReenablesProjectScopeAfterProjectDisable is a full-flow
+// reproduction of a re-enable regression: after `entire disable --project`,
+// running `entire enable --checkpoint-remote ...` with no --project/--local
+// reported success but wrote the enabled flag to .entire/settings.local.json,
+// leaving the project .entire/settings.json the user disabled still
+// enabled=false.
+//
+// Because settings.local.json (enabled:true) overrides settings.json in the
+// merged view that both `entire status` and IsEnabled read, status actually
+// reported ENABLED — the effective state was correct and only the committed
+// file was stale. That still bites anyone without the local file (a fresh
+// clone, a teammate) and leaves the committed source of truth wrong.
+//
+// This drives the real entire binary end-to-end — enable, disable --project,
+// then a setup-flag re-enable — and asserts the PROJECT settings.json (the file
+// the user actually disabled) is enabled again.
+func TestEnableReenablesProjectScopeAfterProjectDisable(t *testing.T) {
+	t.Parallel()
+	env := NewTestEnv(t)
+	defer env.Cleanup()
+
+	env.InitRepo()
+
+	// First-time setup via the real binary; a plain enable writes the project
+	// .entire/settings.json.
+	env.RunCLI("enable", "--agent", "claude-code", "--telemetry=false")
+	assertProjectSettingsEnabled(t, env, true)
+
+	// Disable at the project scope → settings.json enabled=false.
+	env.RunCLI("disable", "--project")
+	assertProjectSettingsEnabled(t, env, false)
+
+	// Re-enable with a setup flag but WITHOUT --project/--local. Pre-fix the
+	// enabled flag landed in settings.local.json, so the project file the user
+	// disabled stayed enabled=false.
+	env.RunCLI("enable", "--checkpoint-remote", "github:org/repo", "--skip-push-sessions", "--telemetry=false")
+
+	assertProjectSettingsEnabled(t, env, true)
+
+	// And no local override may contradict it: settings.local.json must be
+	// absent or itself enabled:true, so the merged view can't silently flip
+	// back to disabled by accident of the flow.
+	assertLocalSettingsAbsentOrEnabled(t, env)
+}
+
+// assertLocalSettingsAbsentOrEnabled asserts that .entire/settings.local.json,
+// if present, does not carry an enabled:false override that would mask the
+// committed project scope.
+func assertLocalSettingsAbsentOrEnabled(t *testing.T, env *TestEnv) {
+	t.Helper()
+	localPath := filepath.Join(env.RepoDir, ".entire", "settings.local.json")
+	data, err := os.ReadFile(localPath)
+	if os.IsNotExist(err) {
+		return
+	}
+	if err != nil {
+		t.Fatalf("read .entire/settings.local.json: %v", err)
+	}
+	var s struct {
+		Enabled *bool `json:"enabled"`
+	}
+	if err := json.Unmarshal(data, &s); err != nil {
+		t.Fatalf("parse .entire/settings.local.json: %v\ncontent: %s", err, data)
+	}
+	if s.Enabled != nil && !*s.Enabled {
+		t.Fatalf("settings.local.json carries enabled:false, which would mask the re-enabled project scope\ncontent: %s", data)
+	}
+}
+
+// assertProjectSettingsEnabled reads .entire/settings.json (the project scope,
+// never settings.local.json) and asserts its enabled flag matches want.
+func assertProjectSettingsEnabled(t *testing.T, env *TestEnv, want bool) {
+	t.Helper()
+	settingsPath := filepath.Join(env.RepoDir, ".entire", "settings.json")
+	data, err := os.ReadFile(settingsPath)
+	if err != nil {
+		t.Fatalf("read .entire/settings.json: %v", err)
+	}
+	var s struct {
+		Enabled bool `json:"enabled"`
+	}
+	if err := json.Unmarshal(data, &s); err != nil {
+		t.Fatalf("parse .entire/settings.json: %v\ncontent: %s", err, data)
+	}
+	if s.Enabled != want {
+		t.Fatalf("project settings.json enabled=%v, want %v — enabled flag written to the wrong scope\ncontent: %s",
+			s.Enabled, want, data)
+	}
+}

cmd/entire/cli/settings/settings.go

@@ -630,6 +630,13 @@ func saveRaw(path, label string, raw map[string]json.RawMessage) error {
 	if err != nil {
 		return fmt.Errorf("marshal %s settings: %w", label, err)
 	}
+	// Ensure the parent directory exists, mirroring the struct save path
+	// (saveToFile). Without this, the raw save path fails in a repo that has
+	// never created .entire/ — e.g. a bare `entire disable` in a fresh repo,
+	// which resolves to a raw flip before any directory is created.
+	if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil {
+		return fmt.Errorf("creating %s settings directory: %w", label, err)
+	}
 	if err := jsonutil.WriteFileAtomic(path, data, 0o644); err != nil {
 		return fmt.Errorf("writing %s settings: %w", label, err)
 	}

cmd/entire/cli/settings/settings_test.go

@@ -1332,6 +1332,45 @@ func TestMergeReviewProfiles_PureAndPrecedence(t *testing.T) {
 	}
 }
 
+// TestSaveProjectRaw_CreatesMissingParentDir verifies the raw save path creates
+// its parent directory, mirroring the struct save path (saveToFile). Without
+// this, a raw enabled-flag flip in a repo that has never created .entire/
+// (e.g. a bare `entire disable` in a fresh repo) hard-fails with "no such file
+// or directory". Regression test for the saveRaw MkdirAll fix.
+func TestSaveProjectRaw_CreatesMissingParentDir(t *testing.T) {
+	tmpDir := t.TempDir()
+	path := filepath.Join(tmpDir, ".entire", "settings.json")
+
+	raw := map[string]json.RawMessage{"enabled": json.RawMessage("false")}
+	if err := SaveProjectRaw(path, raw); err != nil {
+		t.Fatalf("SaveProjectRaw() into a missing .entire dir should succeed, got: %v", err)
+	}
+
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("settings file should have been created: %v", err)
+	}
+	if !strings.Contains(string(data), `"enabled": false`) {
+		t.Errorf("expected enabled:false, got: %s", data)
+	}
+}
+
+// TestSaveLocalRaw_CreatesMissingParentDir is the local-scope mirror of
+// TestSaveProjectRaw_CreatesMissingParentDir.
+func TestSaveLocalRaw_CreatesMissingParentDir(t *testing.T) {
+	tmpDir := t.TempDir()
+	path := filepath.Join(tmpDir, ".entire", "settings.local.json")
+
+	raw := map[string]json.RawMessage{"enabled": json.RawMessage("false")}
+	if err := SaveLocalRaw(path, raw); err != nil {
+		t.Fatalf("SaveLocalRaw() into a missing .entire dir should succeed, got: %v", err)
+	}
+
+	if _, err := os.ReadFile(path); err != nil {
+		t.Fatalf("local settings file should have been created: %v", err)
+	}
+}
+
 // Regression: `entire enable --local` writes only .entire/settings.local.json,
 // but the hook activation check (IsSetUpAndEnabled) only looked for
 // .entire/settings.json, so hooks silently no-op'd. It must recognize a