feat(review): skill-bearing workers get no injected default task · Entire

feat(review): skill-bearing workers get no injected default task

2b493c5·

peyton-alt·1w ago·4 files·+150 added/-5 removed

Product intent: entire invokes the configured skills in parallel across agents — it does not add prompts of its own unless the user configured them. Previously an empty profile task was silently filled with the built-in comprehensive-audit brief (correctness, regressions, API design, missing tests, maintainability...), and any non-empty Task also dragged in the output-format protocol. A worker configured with only "skills" therefore received a maximal-audit mandate the user never wrote — measured live as the difference between a ~14-minute entire-spawned review and the same skill invoked directly by the user.

Per-worker task resolution: a user-configured task always reaches every worker verbatim; with no user task, skill-bearing workers get an empty Task (skills define behavior — and no Task means no injected output protocol either), while skill-less workers keep the built-in default as fallback so their prompt isn't empty. The judge still receives the canonical profile task for consolidation.

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

Sessions

1e2563ad0061View transcript

[?
# Handoff To Claude: entire review RedoClaude Code·30 steps](/content/gh/entireio/cli/session/93833a17-c2c6-4cb0-85b2-663c867b105f#timeline-1e2563ad0061/index.html)

Changes

4

847 unmodified lines

848
849
850
851
852
853
854
855
856
857
9 unmodified lines

867
868
869
866
870
871
872
873
12 unmodified lines

886
887
888
885
889
890
891
892
14 unmodified lines

907
908
909
906
910
911
912
913
225 unmodified lines

1139
1140
1141
1142
1143
1144
1145
59 unmodified lines

1205
1206
1207
1203
1208
1209
1210
1211

847 unmodified lines

fmt.Fprintln(cmd.ErrOrStderr(), err.Error())
    return silentErr(err)
}
// userTask is only what the user configured; profile.Task becomes the
// canonical task for the judge/synthesis phase. Workers resolve their own
// brief via workerTask — skill-bearing workers get no injected default.
userTask := strings.TrimSpace(profile.Task)
profile.Task = profileTask(profileName, profile)
profile.Agents = nonZeroAgentConfigs(profile.Agents)
outputMode := profileOutput(profile)
9 unmodified lines

if modelOverride != "" {
        cfg.Model = modelOverride
    }
    return runSingleAgentPath(ctx, cmd, profileName, workerName, baseOverride, perRunPrompt, profile.Task, outputMode, timeout, cfg, installed, deps, out)
    return runSingleAgentPath(ctx, cmd, profileName, workerName, baseOverride, perRunPrompt, workerTask(profileName, userTask, cfg), outputMode, timeout, cfg, installed, deps, out)
}

if missing := missingInstalledProfileAgents(profile.Agents, installed); len(missing) > 0 {
12 unmodified lines

return silentErr(err)
case 1:
    cfg := profile.Agents[eligible[0].Name]
    return runSingleAgentPath(ctx, cmd, profileName, eligible[0].Name, baseOverride, perRunPrompt, profile.Task, outputMode, timeout, cfg, installed, deps, out)
    return runSingleAgentPath(ctx, cmd, profileName, eligible[0].Name, baseOverride, perRunPrompt, workerTask(profileName, userTask, cfg), outputMode, timeout, cfg, installed, deps, out)
default:
    launchableEligible := computeLaunchableEligibleForProfile(profile, installed, deps.ReviewerFor)
    if len(launchableEligible) != len(eligible) {
14 unmodified lines

fmt.Fprintln(cmd.ErrOrStderr(), err.Error())
        return silentErr(err)
    }
    return runMultiAgentPath(ctx, cmd, profileName, profile, launchableEligible, judge, outputMode, timeout, baseOverride, perRunPrompt, deps, out)
    return runMultiAgentPath(ctx, cmd, profileName, profile, userTask, launchableEligible, judge, outputMode, timeout, baseOverride, perRunPrompt, deps, out)
}
}

225 unmodified lines

cmd *cobra.Command,
    profileName string,
    profile settings.ReviewProfileConfig,
    userTask string,
    launchableEligible []AgentChoice,
    judge judgeSpec,
    outputMode string,
59 unmodified lines

inner: reviewer,
        cfg: runConfigWithReviewConfig(reviewtypes.RunConfig{
            ProfileName:       profileName,
            Task:              profile.Task,
            Task:              workerTask(profileName, userTask, agentCfg),
            PerRunPrompt:      perRunPrompt,
            ScopeBaseRef:      scopeBaseRef,
            CheckpointContext: checkpointContext,

Mcmd/entire/cli/review/cmd.go+9/-4

68 unmodified lines

69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
1240 unmodified lines

1357
1358
1359
1360
1361
1362
1363
1364
1365
1366
1367
1368
1369
1370
1371
1372
1373
1374
1375
1376
1377
1378
1379
1380
1381
1382
1383
1384
1385
1386
1387
1388
1389
1390
1391
1392
1393
1394
1395
1396
1397
1398
1399
1400
1401
1402
1403
1404
1405
1406
1407
1408
1409
1410
1411
1412
1413
1414
1415
1416
1417
1418
1419
1420
1421
1422
1423
1424
1425
1426
1427
1428
1429
1430
1431
1432
1433
1434
1435
1436
1437
1438
1439
1440

68 unmodified lines

return settings.SaveClonePreferences(ctx, prefs)
}
// seedReviewProfile persists an explicit profile into clone-local
// preferences — unlike seedReviewConfig it does not invent a task, so tests
// can distinguish user-configured tasks from entire-injected defaults.
func seedReviewProfile(ctx context.Context, profile settings.ReviewProfileConfig) error {
    prefs, err := settings.LoadClonePreferences(ctx)
    if err != nil {
        return err
    }
    if prefs == nil {
        prefs = &settings.ClonePreferences{}
    }
    prefs.ReviewDefaultProfile = review.DefaultProfileName
    if profile.Judge == nil {
        if judge := defaultTestJudge(profile.Agents); judge != "" {
            profile.Judge = &settings.ReviewConfig{Agent: judge}
        }
    }
    prefs.ReviewProfiles = map[string]settings.ReviewProfileConfig{
        review.DefaultProfileName: profile,
    }
    return settings.SaveClonePreferences(ctx, prefs)
}

// newCaptureDeps builds minimal Deps that route every launch to reviewer.
func newCaptureDeps(reviewer *captureRunConfigReviewer) review.Deps {
    return review.Deps{
        GetAgentsWithHooksInstalled: func(_ context.Context) []types.AgentName {
            return []types.AgentName{types.AgentName(reviewer.name)}
        },
        NewSilentError: func(err error) error { return err },
        HeadHasReviewCheckpoint: func(_ context.Context) (bool, string) {
            return false, ""
        },
        ReviewerFor: func(agentName string) reviewtypes.AgentReviewer {
            if agentName == reviewer.name {
                return reviewer
            }
            return nil
        },
    }
}

func defaultTestJudge(cfg map[string]settings.ReviewConfig) string {
    if _, ok := cfg[string(agent.AgentNameClaudeCode)]; ok {
        return string(agent.AgentNameClaudeCode)
    }
}

// TestRunReview_SkillWorkersGetNoInjectedTask encodes the product intent:
// entire invokes the configured skills; it does not add its own review brief
// unless the user configured one. The built-in default task (and the output
// format protocol that rides on a non-empty Task) ordered a maximal audit
// that made entire-spawned reviews far heavier than the same skill invoked
// by the user directly.
func TestRunReview_SkillWorkersGetNoInjectedTask(t *testing.T) {
    setupCmdTestRepo(t)
    if err := seedReviewProfile(context.Background(), settings.ReviewProfileConfig{
        Agents: map[string]settings.ReviewConfig{testAgentName: {Skills: []string{"/review"}}},
    }); err != nil {
        t.Fatal(err)
    }
    
    reviewer := &captureRunConfigReviewer{name: testAgentName}
    deps := newCaptureDeps(reviewer)

cmd := review.NewCommand(deps)
    cmd.SetOut(&bytes.Buffer{})
    cmd.SetErr(&bytes.Buffer{})
    cmd.SetArgs([]string{"general"})
    if err := cmd.Execute(); err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
    if reviewer.got.Task != "" {
        t.Errorf("skill-bearing worker got injected Task %q, want empty (skills define behavior)", reviewer.got.Task)
    }
}

// TestRunReview_UserConfiguredTaskAlwaysIncluded verifies an explicit task in
// the profile reaches skill-bearing workers verbatim.
func TestRunReview_UserConfiguredTaskAlwaysIncluded(t *testing.T) {
    setupCmdTestRepo(t)
    if err := seedReviewProfile(context.Background(), settings.ReviewProfileConfig{
        Task:   "Focus on the storage layer only.",
        Agents: map[string]settings.ReviewConfig{testAgentName: {Skills: []string{"/review"}}},
    }); err != nil {
        t.Fatal(err)
    }
    
    reviewer := &captureRunConfigReviewer{name: testAgentName}
    deps := newCaptureDeps(reviewer)

cmd := review.NewCommand(deps)
    cmd.SetOut(&bytes.Buffer{})
    cmd.SetErr(&bytes.Buffer{})
    cmd.SetArgs([]string{"general"})
    if err := cmd.Execute(); err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
    if reviewer.got.Task != "Focus on the storage layer only." {
        t.Errorf("Task = %q, want the user-configured task verbatim", reviewer.got.Task)
    }
}

// TestRunReview_SkillLessWorkersKeepDefaultTask verifies the built-in task
// remains the fallback for workers with no skills and no user task — they
// need some brief or the child gets an empty prompt.
func TestRunReview_SkillLessWorkersKeepDefaultTask(t *testing.T) {
    setupCmdTestRepo(t)
    if err := seedReviewProfile(context.Background(), settings.ReviewProfileConfig{
        Agents: map[string]settings.ReviewConfig{testAgentName: {Prompt: "Look carefully."}},
    }); err != nil {
        t.Fatal(err)
    }
    
    reviewer := &captureRunConfigReviewer{name: testAgentName}
    deps := newCaptureDeps(reviewer)

cmd := review.NewCommand(deps)
    cmd.SetOut(&bytes.Buffer{})
    cmd.SetErr(&bytes.Buffer{})
    cmd.SetArgs([]string{"general"})
    if err := cmd.Execute(); err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
    if reviewer.got.Task == "" {
        t.Error("skill-less worker got empty Task, want the built-in default brief")
    }
}