Merge branch 'main' into fix/1743-defer-checkpoint-push-empty-remote · Entire

Merge branch 'main' into fix/1743-defer-checkpoint-push-empty-remote

f12a526→main· karthik-rameshkumar·3d ago·9 files·+574 added/-112 removed

Changes

9

10 unmodified lines
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
63 unmodified lines
92
93
94
88
89
90
91
92
93
94
95
96
95
96
97
98
99
100
101
102
103
104
98
99
100
101
105
106
107
103
108
109
110
111
105
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
110
111
112
113
114
115
116
117
118
119
120
121
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
150
151
152
153
177
178
179
180
181
182
183
184
185
1 unmodified line

187
188
189
161
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211

10 unmodified lines

"strconv"
    "strings"
    "sync"
    "time"

"github.com/entireio/cli/cmd/entire/cli/logging"
    "github.com/entireio/cli/cmd/entire/cli/settings"

// stampConfigTimeout bounds the local git-config reads/writes that mark a newly
// created checkpoint remote as skipped. They run detached from the fetch's
// context (see stampNewlyCreatedRemote), so a bound guards against a stuck
// config lock hanging the caller.
const stampConfigTimeout = 10 * time.Second

// CheckpointTokenEnvVar is the environment variable for providing an access token
// used to authenticate git push/fetch operations for checkpoint branches.
// The token is injected as an HTTP Basic Authorization header per RFC 7617:
63 unmodified lines

args = append(args, opts.Remote)
    args = append(args, opts.RefSpecs...)

cmd := newCommand(ctx, args...)
    if opts.Dir != "" {
        cmd.Dir = opts.Dir
    }
    disableTerminalPrompt(cmd)
    out, err := cmd.CombinedOutput()
    if err != nil {
        return out, fmt.Errorf("git fetch: %w", err)
    }
    // A filtered fetch from a URL makes git record a URL-keyed remote section
    // (remote.<url>.*) so it can lazy-fetch filtered-out objects later. That
    // section also turns the URL into a phantom remote that `git fetch --all`
    // and `git remote update` keep dialing. When this fetch is the one creating
    // the section, stamp skipFetchAll so bulk fetches skip our adhoc remote.
    // Remotes that already existed are left untouched so we never rewrite the
    // user's config.
    var stampURL string
    var stampCandidate, existedBefore bool
    if filtered && IsURL(opts.Remote) {
        // Stamp the URL git actually fetched from: with a checkpoint token set,
        // newCommand rewrites SSH targets to HTTPS, and git records the
        // promisor entry under the rewritten URL.
        target := opts.Remote
        stampCandidate = true
        stampURL = opts.Remote
        if token := strings.TrimSpace(os.Getenv(CheckpointTokenEnvVar)); token != "" && isValidToken(token) {
            target, _ = resolveTargetForTokenAuth(ctx, target)
            // With a checkpoint token, newCommand rewrites SSH targets to HTTPS
            // and git records the section under the rewritten URL.
            stampURL, _ = resolveTargetForTokenAuth(ctx, stampURL)
        }
        markPromisorEntrySkipped(ctx, opts.Dir, target)
        existedBefore = gitRemoteSectionExists(ctx, opts.Dir, stampURL)
    }

cmd := newCommand(ctx, args...)
    if opts.Dir != "" {
        cmd.Dir = opts.Dir
    }
    disableTerminalPrompt(cmd)
    out, err := cmd.CombinedOutput()

if stampCandidate && !existedBefore {
        stampNewlyCreatedRemote(ctx, opts.Dir, stampURL)
    }

if err != nil {
        return out, fmt.Errorf("git fetch: %w", err)
    }
    return out, nil
}

// markPromisorEntrySkipped excludes the URL-keyed config section that git
// creates for a filtered URL fetch (remote.<url>.promisor=true) from
// `git fetch --all` and `git remote update`. Git needs the promisor entry to
// lazy-fetch filtered-out objects later, but the entry also makes the URL show
// up as a fetchable remote, so without this every checkpoint URL ever fetched
// from lingers as a phantom remote that bulk fetches keep dialing.
// Best-effort: the fetch already succeeded, so failures only log.
func markPromisorEntrySkipped(ctx context.Context, dir, url string) {
    if !gitConfigBool(ctx, dir, "remote."+url+".promisor") {
        // Git didn't record a promisor entry for this URL; don't invent a
        // config section that wouldn't otherwise exist.
        return
}
// stampNewlyCreatedRemote stamps a URL-keyed remote section that this fetch just
// created. Git writes remote.<url>.promisor eagerly during connection setup, so
// a filtered fetch that later fails still leaves the phantom remote behind;
// stamping here — rather than only on fetch success — keeps it from lingering
// unstamped forever (the section then exists on the next attempt, so it never
// looks "new" again). Re-checking existence keeps us from inventing a section
// when the fetch died before git wrote anything.
// The git-config commands run on a context detached from the fetch's deadline:
// a filtered fetch that timed out leaves ctx already past its deadline, and
// inheriting it would make these local commands fail immediately and leave the
// phantom unstamped — the very miss this stamping exists to prevent.
func stampNewlyCreatedRemote(ctx context.Context, dir, url string) {
    ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), stampConfigTimeout)
    defer cancel()
    if gitRemoteSectionExists(ctx, dir, url) {
        markRemoteSkipped(ctx, dir, url)
    }
    for _, key := range []string{"skipFetchAll", "skipDefaultUpdate"} {
        fullKey := "remote." + url + "." + key
        if gitConfigBool(ctx, dir, fullKey) {
            // Checked per key so a partially-stamped entry (e.g. an earlier
            // run failing between the two writes) still gets completed.
            continue
        }
        cmd := exec.CommandContext(ctx, "git", "config", "--local", fullKey, "true")
        if dir != "" {
            cmd.Dir = dir
        }
        if out, cfgErr := cmd.CombinedOutput(); cfgErr != nil {
            redactedURL := RedactURL(url)
            // The output can echo the key, which embeds the URL — and a URL
            // can carry credentials. Redact before logging.
            msg := strings.TrimSpace(strings.ReplaceAll(string(out), url, redactedURL))
            logging.Warn(ctx, "failed to mark promisor config entry as skipped for bulk fetches",
                slog.String("url", redactedURL),
                slog.String("key", key),
                slog.String("output", msg),
                slog.String("error", cfgErr.Error()),
            )
            return
        }
}

// markRemoteSkipped stamps skipFetchAll on a URL-keyed remote section so
// `git fetch --all` and `git remote update` skip it. Called only for remotes
// this fetch just created, so an adhoc checkpoint URL never lingers as a phantom
// remote that bulk fetches keep dialing.
// Best-effort: the git config write is not worth failing the fetch over, so
// failures only log.
func markRemoteSkipped(ctx context.Context, dir, url string) {
    fullKey := "remote." + url + ".skipFetchAll"
    cmd := exec.CommandContext(ctx, "git", "config", "--local", fullKey, "true")
    if dir != "" {
        cmd.Dir = dir
    }
    if out, cfgErr := cmd.CombinedOutput(); cfgErr != nil {
        redactedURL := RedactURL(url)
        // The output can echo the key, which embeds the URL — and a URL can
        // carry credentials. Redact before logging.
        msg := strings.TrimSpace(strings.ReplaceAll(string(out), url, redactedURL))
        logging.Warn(ctx, "failed to mark remote config entry as skipped for bulk fetches",
                slog.String("url", redactedURL),
                slog.String("output", msg),
                slog.String("error", cfgErr.Error()),
            )
    }
}

// gitConfigBool reads a local git config key and reports whether it is set to
// a true value. Missing keys and read errors report false.
func gitConfigBool(ctx context.Context, dir, key string) bool {
    cmd := exec.CommandContext(ctx, "git", "config", "--local", "--get", "--type=bool", key)// gitRemoteSectionExists reports whether a remote.<url>.* config section already
// exists in the local git config. Used to tell whether a filtered URL fetch is
// about to create a new URL-keyed remote, so we only stamp remotes we create and
// never rewrite ones the user already has.
func gitRemoteSectionExists(ctx context.Context, dir, url string) bool {
    cmd := exec.CommandContext(ctx, "git", "config", "--local", "--list", "--name-only")
    if dir != "" {
        cmd.Dir = dir
    }
1 unmodified line

if err != nil {
        return false
    }
    return strings.TrimSpace(string(out)) == "true"
    // Each name is "remote.<url>.<key>". Git config keys carry no dots, so the
    // final dotted component is the key and everything between "remote." and it
    // is the subsection (the URL, whose case git preserves). Compare the
    // subsection exactly so a longer URL that shares a prefix (e.g. a
    // ".../repo.git" section vs a ".../repo" fetch) is not a false match.
    for line := range strings.SplitSeq(string(out), "
") {
        rest, ok := strings.CutPrefix(line, "remote.")
        if !ok {
            continue
        }
        lastDot := strings.LastIndexByte(rest, '.')
        if lastDot < 0 {
            continue
        }
        if rest[:lastDot] == url {
            return true
        }
    }
    return false
}

// FetchBlobs fetches specific objects (typically blobs) by hash from a remote.

Mcmd/entire/cli/checkpoint/remote/git.go+103/-56


791 unmodified lines

792
793
794
795
796
797
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
800
814
815
816
817
46 unmodified lines

864
865
866
853
854
867
868
869
2 unmodified lines

872
873
874
875
876
877
878
879
880
881
882
883
884
885
886
887
888
889
890
891
892
893
894
895
896
897
898
899
900
901
902
903
904
905
906
907
908
909
910
911
912
913
914
915
916
917
918
919
920
921
922
923
924
925
864
865
926
927
928
929
930
32 unmodified lines

963
964
965
904
966
967
907
908
909
910
968
969
970
971
972
973
974
975
976
977
978
979
980
981
982
983
984
985
986
987
988
989
990
991
992
993
994
995
996
997
998
999
1000
1001
1002
1003
1004
1005
1006
1007
1008
1009
1010
1011
1012
1013
1014
1015
1016
1017
1018
1019
1020
1021
1022
1023
1024
1025
1 unmodified line

1027
1028
1029
918
919
920
921
1030
1031
1032
924
925
1033
1034
1035
1036
1037
1038
1039
1040
1041
1042
1043
1044
1045
1046
1047
1048
1049
1050
1051
1052
1053
1054
1055
1056
1057
1058
1059
1060
1061
1062
1063
1064
1065
1066
1067
1068
1069
1070
1071
1072
1073
1074
1075
1076
1077
1078
1079
1080
1081
1082
1083
1084
1085
1086
1087
1088
1089
1090
1091

791 unmodified lines

return m

// TestFetch_FilteredURLFetchMarksPromisorSkipped verifies that after a
// filtered fetch from a URL, the URL-keyed promisor config section git creates
// is excluded from `git fetch --all` / `git remote update` — otherwise every
// gitConfigBool reads a local git config key and reports whether it is set to a
// true value. Missing keys and read errors report false.
func gitConfigBool(ctx context.Context, dir, key string) bool {
    cmd := exec.CommandContext(ctx, "git", "config", "--local", "--get", "--type=bool", key)
    if dir != "" {
        cmd.Dir = dir
    }
    out, err := cmd.Output()
    if err != nil {
        return false
    }
    return strings.TrimSpace(string(out)) == "true"
}

// TestFetch_FilteredURLFetchMarksNewRemoteSkipped verifies that when a filtered
// fetch from a URL creates a new URL-keyed remote section, that section is
// excluded from `git fetch --all` / `git remote update` — otherwise every
// checkpoint URL ever fetched from lingers as a phantom remote that bulk
// fetches keep dialing.
func TestFetch_FilteredURLFetchMarksPromisorSkipped(t *testing.T) {
func TestFetch_FilteredURLFetchMarksNewRemoteSkipped(t *testing.T) {
    ctx := context.Background()

tmpDir := t.TempDir()
46 unmodified lines

assert.True(t, gitConfigBool(ctx, cloneDir, "remote."+fetchURL+".skipFetchAll"),
        "URL-keyed promisor entry should be excluded from git fetch --all")
    assert.True(t, gitConfigBool(ctx, cloneDir, "remote."+fetchURL+".skipDefaultUpdate"),
        "URL-keyed promisor entry should be excluded from git remote update")

// git fetch --all must no longer dial the phantom entry: with the
    // checkpoint repo gone, --all only succeeds if the URL-keyed entry is
2 unmodified lines

runIsolatedGit(ctx, t, cloneDir, "fetch", "--all", "--no-auto-gc")
}

// TestFetch_FailedFilteredFetchStillStampsNewRemote guards the resume
// regression: git writes remote.<url>.promisor eagerly during connection
// setup, so a filtered fetch that then fails (e.g. a missing ref) still leaves
// the phantom remote behind. The stamp must land anyway — otherwise the section
// exists on the next attempt, never looks new again, and lingers unstamped.
func TestFetch_FailedFilteredFetchStillStampsNewRemote(t *testing.T) {
    ctx := context.Background()

tmpDir := t.TempDir()
    originBare := filepath.Join(tmpDir, "origin.git")
    checkpointBare := filepath.Join(tmpDir, "checkpoints.git")
    seedDir := filepath.Join(tmpDir, "seed")
    cloneDir := filepath.Join(tmpDir, "clone")

testutil.InitRepo(t, seedDir)
    testutil.WriteFile(t, seedDir, "f.txt", "init")
    testutil.GitAdd(t, seedDir, "f.txt")
    testutil.GitCommit(t, seedDir, "init")

runIsolatedGit(ctx, t, "", "init", "--bare", originBare)
    runIsolatedGit(ctx, t, "", "init", "--bare", checkpointBare)
    runIsolatedGit(ctx, t, checkpointBare, "config", "uploadpack.allowFilter", "true")
    runIsolatedGit(ctx, t, seedDir, "push", originBare, "HEAD:refs/heads/main")
    runIsolatedGit(ctx, t, "", "clone", "--branch", "main", "file://"+originBare, cloneDir)

testutil.WriteFile(
        t, cloneDir,
        ".entire/settings.json",
        `{"enabled": true, "strategy_options": {"filtered_fetches": true}}`,
    )
    t.Chdir(cloneDir)

fetchURL := "file://" + checkpointBare
    // Fetch a ref that does not exist on the checkpoint remote: the command
    // fails, but git has already recorded the URL-keyed promisor section.
    _, err := Fetch(ctx, FetchOptions{
        Remote:   fetchURL,
        RefSpecs: []string{"+refs/heads/does-not-exist:refs/entire-fetch-tmp/x"},
        NoTags:   true,
        Dir:      cloneDir,
    })
    require.Error(t, err, "fetch of a missing ref should fail")

require.True(t, gitConfigBool(ctx, cloneDir, "remote."+fetchURL+".promisor"),
        "git records the promisor section even when the fetch fails")
    assert.True(t, gitConfigBool(ctx, cloneDir, "remote."+fetchURL+".skipFetchAll"),
        "a phantom remote left by a failed fetch must still be stamped")
}

// TestFetch_UnfilteredFetchDoesNotCreateConfigSection verifies the stamp is
// gated on git having created a promisor entry: a plain (unfiltered) URL fetch
// must not invent a remote.<url> config section.
// gated on a filtered fetch: a plain (unfiltered) URL fetch records no
// URL-keyed section, so we must not invent a remote.<url> config section.
func TestFetch_UnfilteredFetchDoesNotCreateConfigSection(t *testing.T) {
    ctx := context.Background()

assert.False(t, gitConfigBool(ctx, cloneDir, "remote."+fetchURL+".promisor"))
    assert.False(t, gitConfigBool(ctx, cloneDir, "remote."+fetchURL+".skipFetchAll"))
    assert.False(t, gitConfigBool(ctx, cloneDir, "remote."+fetchURL+".skipDefaultUpdate"))
}

// TestMarkPromisorEntrySkipped_CompletesPartialStamp verifies the keys are
// checked independently: an entry with skipFetchAll already set (e.g. an
// earlier run failing between the two writes) still gets skipDefaultUpdate.
func TestMarkPromisorEntrySkipped_CompletesPartialStamp(t *testing.T) {
// TestFetch_ExistingURLRemoteNotReStamped verifies we only stamp remotes we
// create: a filtered fetch from a URL that already has a remote.<url> section
// must leave that section as-is rather than rewriting the user's git config.
func TestFetch_ExistingURLRemoteNotReStamped(t *testing.T) {
    ctx := context.Background()

testutil.InitRepo(t, seedDir)
    testutil.WriteFile(t, seedDir, "f.txt", "init")
    testutil.GitAdd(t, seedDir, "f.txt")
    testutil.GitCommit(t, seedDir, "init")

testutil.WriteFile(t, seedDir, "f.txt", "init\next\")
    testutil.GitAdd(t, seedDir, "f.txt")
    testutil.GitCommit(t, seedDir, "next")
     runIsolatedGit(ctx, t, seedDir, "push", checkpointBare, "HEAD:refs/heads/main")

testutil.WriteFile(
        t, cloneDir,
        ".entire/settings.json",
        `{"enabled": true, "strategy_options": {"filtered_fetches": true}}`,
    )
    t.Chdir(cloneDir)

fetchURL := "file://" + checkpointBare
    // Simulate a pre-existing URL-keyed remote (e.g. a phantom left by an older
    // CLI). Its presence means the section already exists before our fetch.
    runIsolatedGit(ctx, t, cloneDir, "config", "--local", "remote."+fetchURL+".promisor", "true")

out, err := Fetch(ctx, FetchOptions{
        Remote:   fetchURL,
        RefSpecs: []string{"+refs/heads/main:refs/entire-fetch-tmp/main"},
        NoTags:   true,
        Dir:      cloneDir,
    })
    require.NoError(t, err, "fetch output: %s", out)

assert.False(t, gitConfigBool(ctx, cloneDir, "remote."+fetchURL+".skipFetchAll"),
        "a remote that already existed must not be stamped")
}

// TestMarkRemoteSkipped_SetsSkipFetchAll verifies the helper stamps skipFetchAll.
func TestMarkRemoteSkipped_SetsSkipFetchAll(t *testing.T) {
    t.Parallel()
    ctx := context.Background()

testutil.InitRepo(t, repoDir)

const url = "https://example.com/org/checkpoints.git"
    runIsolatedGit(ctx, t, repoDir, "config", "--local", "remote."+url+".promisor", "true")
    runIsolatedGit(ctx, t, repoDir, "config", "--local", "remote."+url+".skipFetchAll", "true")

markPromisorEntrySkipped(ctx, repoDir, url)
    markRemoteSkipped(ctx, repoDir, url)

assert.True(t, gitConfigBool(ctx, repoDir, "remote."+url+".skipFetchAll"))
    assert.True(t, gitConfigBool(ctx, repoDir, "remote."+url+".skipDefaultUpdate"),
        "partially-stamped entry should be completed")
}

// TestGitRemoteSectionExists reports true only once a remote.<url>.* key is set.
func TestGitRemoteSectionExists(t *testing.T) {
    t.Parallel()
    ctx := context.Background()

repoDir := t.TempDir()
    testutil.InitRepo(t, repoDir)

const url = "https://example.com/org/checkpoints.git"
    assert.False(t, gitRemoteSectionExists(ctx, repoDir, url))

runIsolatedGit(ctx, t, repoDir, "config", "--local", "remote."+url+".promisor", "true")
    assert.True(t, gitRemoteSectionExists(ctx, repoDir, url),
        "the exact URL section is present")
    assert.False(t, gitRemoteSectionExists(ctx, repoDir, shortURL),
        "a prefix of an existing URL section must not count as present")
}

// TestStampNewlyCreatedRemote_StampsUnderCancelledContext guards the timed-out
// fetch case: git records the promisor section before the fetch times out, so
// the stamp must still land even though the fetch context is already cancelled.
func TestStampNewlyCreatedRemote_StampsUnderCancelledContext(t *testing.T) {
    t.Parallel()

repoDir := t.TempDir()
    testutil.InitRepo(t, repoDir)

const url = "https://example.com/org/checkpoints.git"
    // Simulate git having recorded the promisor section during a fetch that
    // then timed out.
    runIsolatedGit(context.Background(), t, repoDir, "config", "--local", "remote."+url+".promisor", "true")

ctx, cancel := context.WithCancel(context.Background())
    cancel() // parent context already done, as after a timed-out fetch

stampNewlyCreatedRemote(ctx, repoDir, url)

assert.True(t, gitConfigBool(context.Background(), repoDir, "remote."+url+".skipFetchAll"),
        "stamp must land even though the parent context is cancelled")
}

Mcmd/entire/cli/review/cmd.go+51/-16


1237 unmodified lines

1238
1239
1240
1241
1242
1243
1244
1245
1246
1247
1248
1249
1250
1251
1252
1253
1254
1255
1256
1257
1258
1259
1260
1261
1262
1263
1264
1265
1266
1267
1268
1269
1270
1271
1272
1273
1274
1275
1276
1277
1278
1279
1280
1281
1282
1283
1284
1285
1286
1287
1288
1289
1290
1291
1292
1293
1294
1295
1296
1297
1298
1299
1300
1301
1302
1303
1304
1243
1244
1245
1246
1247
1305
1306
1307
1308
1309
1250
1251
1310
1311
1312
1313
1314
2 unmodified lines

1317
1318
1319
1260
1320
1321
1322
1323
37 unmodified lines

1361
1362
1363
1304
1364
1365
1366
1367
6 unmodified lines

1374
1375
1376
1317
1377
1378
1379
1380

1237 unmodified lines

}
}

// TestDispatchFork_LegacyGeneratedCodexSkillIsRepairedAndLaunched prevents
// guided setup's historical /review default from silently removing Codex from
// a multi-agent run. The compatibility repair must reach dispatch, not merely
// make the profile look valid in listing/configuration code.
func TestDispatchFork_LegacyGeneratedCodexSkillIsRepairedAndLaunched(t *testing.T) {
    setupCmdTestRepo(t)
    t.Setenv("HOME", t.TempDir())

if err := seedReviewConfig(context.Background(), map[string]settings.ReviewConfig{
        testAgentName: {Skills: []string{"/review"}},
        testCodexAgent: {
            Skills: []string{"/review"},
        },
    }); err != nil {
        t.Fatal(err)
    }

claudeReviewer := &captureRunConfigReviewer{name: testAgentName}
    codexReviewer := &captureRunConfigReviewer{name: testCodexAgent}
    deps := review.Deps{
        GetAgentsWithHooksInstalled: func(_ context.Context) []types.AgentName {
            return []types.AgentName{testAgentName, testCodexAgent}
        },
        NewSilentError: func(err error) error { return err },
        HeadHasReviewCheckpoint: func(_ context.Context) (bool, string) {
            return false, ""
        },
        ReviewerFor: func(agentName string) reviewtypes.AgentReviewer {
            switch agentName {
            case testAgentName:
                return claudeReviewer
            case testCodexAgent:
                return codexReviewer
            default:
                return nil
            }
        },
    }

cmd := review.NewCommand(deps)
    cmd.SetOut(&bytes.Buffer{})
    errBuf := &bytes.Buffer{}
    cmd.SetErr(errBuf)
    cmd.SetArgs([]string{"general"})

if err := cmd.Execute(); err != nil {
        t.Fatalf("run legacy generated profile: %v", err)
    }
    if !codexReviewer.called {
        t.Fatalf("Codex was silently excluded; stderr:\n%s", errBuf.String())
    }
    if len(codexReviewer.got.Skills) != 0 {
        t.Fatalf("Codex received obsolete generated skills %v, want none", codexReviewer.got.Skills)
    }
    if codexReviewer.got.AlwaysPrompt != "Review the change according to the profile task." {
        t.Fatalf("Codex repaired prompt = %q", codexReviewer.got.AlwaysPrompt)
    }
    if strings.Contains(errBuf.String(), "skipping reviewer codex") {
        t.Fatalf("Codex was reported as skipped:\n%s", errBuf.String())
    }
}

// TestDispatchFork_InvalidSkillExcludesWorkerNotWholeCrew pins the blast
// radius of spawn-time skill validation in multi-agent runs: a worker whose
// configured skill no longer validates (e.g. codex's legacy auto-preselected
// "/review", orphaned when the curated builtin was removed) is excluded with
// a loud warning, and the remaining reviewers still run. Aborting the whole
// crew for one stale entry held every other agent hostage to a codex
// reconfigure.
// explicitly configured skill no longer validates is excluded with a loud
// warning, and the remaining reviewers still run. Aborting the whole crew for
// one stale entry would hold every other agent hostage to a reconfigure.
func TestDispatchFork_InvalidSkillExcludesWorkerNotWholeCrew(t *testing.T) {
    setupCmdTestRepo(t)
    // Controlled empty HOME: codex discovery finds nothing, so its "/review"
    // (no longer a curated builtin) fails validation. Cannot t.Parallel —
    // Controlled empty HOME: Codex discovery finds nothing, so the configured
    // custom skill fails validation. Cannot t.Parallel —
    // t.Setenv (setupCmdTestRepo already precludes it via t.Chdir).
    t.Setenv("HOME", t.TempDir())

2 unmodified lines

Skills: []string{"/review"},
            testCodexAgent: {
                Skills: []string{"$missing-review"},
            },
        })
    }
}