fix(review): stop persisting the built-in task into saved profiles

fd7d1cb·

peyton-alt·1w ago·4 files·+44 added/-2 removed

Both setup paths (guided picker fallback and non-interactive first-run) wrote the built-in default brief into the saved profile's task field, making it permanently indistinguishable from a task the user wrote — which defeated the skill-workers-get-no-injected-task rule for every profile created by setup. Persist empty; the built-in brief stays a runtime fallback for skill-less workers and the judge.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

53e0eeb2e399View transcript

Changes

4

• cmd/entire/cli/review

• Mhelpers_internal_test.go+25

• Mpicker.go+4/-1

• Mpicker_internal_test.go+13

• Mprofile.go+2/-1

169 unmodified lines

170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197

169 unmodified lines

t.Errorf("dedupeStrings(nil) = %v, want nil", got)
}

// TestDefaultReviewProfile_NoTaskPersisted verifies the non-interactive
// first-run profile leaves task empty — the built-in brief is a runtime
// fallback for skill-less workers, not saved user configuration.
func TestDefaultReviewProfile_NoTaskPersisted(t *testing.T) {
    t.Parallel()
    profile, err := defaultReviewProfileForInstalledAgents(
        context.Background(), DefaultProfileName,
        []types.AgentName{agent.AgentNameClaudeCode},
        func(string) reviewtypes.AgentReviewer { return stubReviewerForProfileTest{} },
    )
    if err != nil {
        t.Fatalf("defaultReviewProfileForInstalledAgents: %v", err)
    }
    if profile.Task != "" {
        t.Errorf("persisted Task = %q, want empty (built-in brief is runtime fallback only)", profile.Task)
    }
}

type stubReviewerForProfileTest struct{}

func (stubReviewerForProfileTest) Name() string { return "stub" }
func (stubReviewerForProfileTest) Start(context.Context, reviewtypes.RunConfig) (reviewtypes.Process, error) {
    return nil, errors.New("not started in tests")
}

Mcmd/entire/cli/review/helpers_internal_test.go+25

170 unmodified lines

171
172
173
174
174
175
176
177
178
179
180

170 unmodified lines

if strings.TrimSpace(generatedTask) != "" {
        return generatedTask
    }
    return profileTask(profileName, settings.ReviewProfileConfig{})
    // No user-provided task: persist empty. The built-in brief is a runtime
    // fallback (workerTask/profileTask), not saved configuration — persisting
    // it would make it indistinguishable from a task the user wrote.
    return ""
}

func launchableInstalledAgentNames(installed []types.AgentName, reviewerFor func(string) reviewtypes.AgentReviewer) []string {

Mcmd/entire/cli/review/picker.go+4/-1

63 unmodified lines

64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79

63 unmodified lines

t.Fatal("custom model option missing")
}

// TestGuidedProfileTask_NoBuiltinFallbackPersisted verifies setup never bakes
// the built-in default brief into the saved profile: with no custom, existing,
// or generated task, the persisted task stays empty and the runtime fallback
// (workerTask / profileTask) supplies defaults where needed. Persisting the
// built-in text made it indistinguishable from a user-configured task, so
// skill-bearing workers kept receiving the maximal-audit brief forever.
func TestGuidedProfileTask_NoBuiltinFallbackPersisted(t *testing.T) {
    t.Parallel()
    if got := guidedProfileTask(DefaultProfileName, "", "", ""); got != "" {
        t.Fatalf("guidedProfileTask with nothing user-provided = %q, want empty", got)
    }
}

Mcmd/entire/cli/review/picker_internal_test.go+13

345 unmodified lines

346
347
348
349
350
351
350
352
353
354

345 unmodified lines

if len(agents) == 0 {
        return settings.ReviewProfileConfig{}, errors.New("no agents with review runner adapters and hooks installed; run `entire configure --agent claude-code`, `entire configure --agent codex`, `entire configure --agent gemini`, or `entire configure --agent pi`")
    }
    // Task deliberately left empty: the built-in brief is a runtime fallback
    // for skill-less workers, not user configuration to persist.
    profile := settings.ReviewProfileConfig{
        Task:   profileTask(profileName, settings.ReviewProfileConfig{}),
        Agents: agents,
    }
    if j, ok := defaultJudge(ctx, agents); ok {