fix(setup): recover legacy #1140 split enable state

53fc718→main·suhaanthayyil·3d ago·2 files·+131 added/-13 removed

entire enable early-returned on the merged IsEnabled view, so it could not recover the split state a pre-fix binary left on disk: committed settings.json enabled:false masked by settings.local.json enabled:true (local wins in the merge). The merged view read enabled, the early return fired, and the committed project file the user cares about stayed disabled forever — even with an explicit --project. This is issue #1140 step 4.

Resolve the target scope first and only short-circuit when the merged view is enabled AND the resolved target file is not itself explicitly disabled (new scopeExplicitlyDisabled helper reads the file's own raw enabled key). Otherwise fall through and flip the target scope.

Changes

2

• cmd/entire/cli
  • Msetup.go+49/-13
  • Msetup_test.go+82

1115 unmodified lines

1116
1117
1118
1119
1120
1121
1122
1123
1124
1125
1126
1127
1128
1129
1130
1119
1120
1121
1122
1123
1124
1125
1126
1134
1127
1128
1129
1130
1131
1132
1133
1134
1135
1136
1137
1138
1139
1140
1141
1142
1143
1144
1145
1146
1147
1148
1149
1150
1151
1152
1153
1154
1155
1156
1157
1158
1159
1160
1161
1162
1163
1164
1165
1166
1167
1168
1169
1170
1171
1172
1173

1115 unmodified lines
}

enabled, err := IsEnabled(ctx)
if err == nil && enabled {
    if !usedSetupFlow {
        fmt.Fprintln(w, "Entire is already enabled.")
    }
    printEnabledStatus(ctx, w)
    return nil
}
// Enable in the same settings target scope resolved by settingsTargetFile,
// which is also what strategy/checkpoint-backend updates above use. Without this,
// a plain `entire enable` (no --project/--local) resolved the strategy
// write to the existing project settings.json but wrote the enabled flag to
// settings.local.json, leaving the project file the user disabled still
// enabled=false (#1140).
targetFile, _ := settingsTargetFile(ctx, opts.UseLocalSettings, opts.UseProjectSettings)
return runEnable(ctx, w, targetFile == settings.EntireSettingsFile)
useProject := targetFile == settings.EntireSettingsFile

// The merged view can report enabled while the resolved target file is
// itself still disabled — exactly the legacy #1140 split state a pre-fix
// binary left on disk (committed settings.json enabled:false masked by
// settings.local.json enabled:true, which wins in the merge). In that case
// the early "already enabled" return would never flip the target file, even
// with an explicit --project, so `enable` could not recover the state #1140
// reports. Only short-circuit when the merged view is enabled AND the target
// file is not itself explicitly disabled.
enabled, err := IsEnabled(ctx)
if err == nil && enabled && !scopeExplicitlyDisabled(ctx, useProject) {
    if !usedSetupFlow {
        fmt.Fprintln(w, "Entire is already enabled.")
    }
    printEnabledStatus(ctx, w)
    return nil
}
return runEnable(ctx, w, useProject)
}

// scopeExplicitlyDisabled reports whether the settings file for the given scope
// exists and carries an explicit "enabled": false. A missing file or a missing
// "enabled" key returns false: those default to enabled, so there is nothing to
// recover. Used to detect the legacy #1140 split state where the merged view is
// enabled but the target file the user cares about is still disabled.
func scopeExplicitlyDisabled(ctx context.Context, useProject bool) bool {
    load := settings.LoadLocalRaw
    if useProject {
        load = settings.LoadProjectRaw
    }
    _, raw, _, err := load(ctx)
    if err != nil {
        return false
    }
    value, ok := raw["enabled"]
    if !ok {
        return false
    }
    var enabled bool
    if err := json.Unmarshal(value, &enabled); err != nil {
        return false
    }
    return !enabled
}

func runEnableInteractive(ctx context.Context, w io.Writer, agents []agent.Agent, opts EnableOptions) error {

Mcmd/entire/cli/setup.go+49/-13

224 unmodified lines

225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312

224 unmodified lines
}

// TestRunEnableOnConfiguredRepo_RecoversLegacySplitState covers issue #1140
// step 4: recovering the split state a pre-fix binary left on disk — committed
// settings.json enabled:false, settings.local.json enabled:true. The local
// override wins in the merged view, so IsEnabled reports true; a bare early
// return on the merged view would leave the committed project file disabled
// forever, even with an explicit --project. runEnableOnConfiguredRepo must
// detect that the target scope is itself disabled and flip it.
func TestRunEnableOnConfiguredRepo_RecoversLegacySplitState(t *testing.T) {
    setupTestRepo(t)
    // Legacy #1140 split state.
    writeSettings(t, testSettingsDisabled)
    writeLocalSettings(t, `{\"enabled\": true}`)

// Sanity: the merged view already reports enabled (local override wins).
    enabled, err := IsEnabled(context.Background())
    if err != nil {
        t.Fatalf("IsEnabled() error = %v", err)
    }
    if !enabled {
        t.Fatal("precondition: merged view should report enabled (local override wins)")
    }

cmd := newEnableCmd()
    var buf bytes.Buffer
    cmd.SetOut(&buf)
    if err := runEnableOnConfiguredRepo(context.Background(), cmd, EnableOptions{UseProjectSettings: true}); err != nil {
        t.Fatalf("runEnableOnConfiguredRepo(--project) error = %v", err)
    }

// The committed project file must now be enabled — the state #1140 could
    // not recover before this fix.
    projectS, err := settings.LoadFromFile(EntireSettingsFile)
    if err != nil {
        t.Fatalf("failed to load project settings: %v", err)
    }
    if !projectS.Enabled {
        t.Error("committed settings.json should be enabled:true after enable --project recovered the split state")
    }
}

// TestRunEnableOnConfiguredRepo_BareEnable_RecoversLegacySplitState verifies the
// same recovery happens for a bare `entire enable` (no --project), which
// resolves to the committed settings.json via settingsTargetFile.
func TestRunEnableOnConfiguredRepo_BareEnable_RecoversLegacySplitState(t *testing.T) {
    setupTestRepo(t)
    writeSettings(t, testSettingsDisabled)
    writeLocalSettings(t, `{\"enabled\": true}`)

cmd := newEnableCmd()
    var buf bytes.Buffer
    cmd.SetOut(&buf)
    if err := runEnableOnConfiguredRepo(context.Background(), cmd, EnableOptions{}); err != nil {
        t.Fatalf("runEnableOnConfiguredRepo() error = %v", err)
    }

projectS, err := settings.LoadFromFile(EntireSettingsFile)
    if err != nil {
        t.Fatalf("failed to load project settings: %v", err)
    }
    if !projectS.Enabled {
        t.Error("committed settings.json should be enabled:true after a bare enable recovered the split state")
    }
}

// TestRunEnableOnConfiguredRepo_AlreadyEnabled_NoSplit verifies the early
// return still fires (nothing to flip, "already enabled") when the merged view
// AND the resolved target scope agree that Entire is enabled.
func TestRunEnableOnConfiguredRepo_AlreadyEnabled_NoSplit(t *testing.T) {
    setupTestRepo(t)
    writeSettings(t, testSettingsEnabled)

cmd := newEnableCmd()
    var buf bytes.Buffer
    cmd.SetOut(&buf)
    if err := runEnableOnConfiguredRepo(context.Background(), cmd, EnableOptions{}); err != nil {
        t.Fatalf("runEnableOnConfiguredRepo() error = %v", err)
    }
    if !strings.Contains(buf.String(), "already enabled") {
        t.Errorf("expected 'already enabled' output when nothing to recover, got: %s", buf.String())
    }
}

// TestRunEnable_ProjectFlag_ClearsLocalDisable verifies that `entire enable --project`
// after `entire disable` (which writes to local) actually re-enables by updating both files.
func TestRunEnable_ProjectFlag_ClearsLocalDisable(t *testing.T) {