Merge branch 'main' into fix/1036-headless-login-hint · Entire
Log in
Merge branch 'main' into fix/1036-headless-login-hint
14f45ea→main·
gtrrz-victor·2d ago·30 files·+753 added/-202 removed
Changes
30
cmd/entire/cli
checkpoint
Mcheckpoint_test.go+1/-1
Mpersistent.go+16/-14
Mpersistent_opf_trailer_test.go+3/-3
Mprompts.go+3/-2
Mprompts_test.go+3/-2
remote
Mgit.go+103/-56
Mgit_test.go+184/-19
integration_test
Mtestenv.go+3/-4
interactive
Minteractive.go+11
review
Mcmd.go+51/-16
Mcmd_test.go+70/-10
Mconfigure_test.go+120
Mfix.go+1/-2
Mprofile.go+31/-5
settings
Msettings.go+1/-1
strategy
Mcommon.go+1/-1
Mmanual_commit_condensation.go+7/-5
Mmanual_commit_hooks.go+4/-4
Mmanual_commit_opf_prompt.go+2/-2
Mmanual_commit_opf_rewrite.go+11/-11
Mmanual_commit_opf_rewrite_test.go+2/-2
Mmanual_commit_push.go+4/-4
trailers
Mtrailers.go+2/-1
Mtrailers_test.go+3/-2
redact
Mbatch.go+12/-11
Mbatch_test.go+7/-7
Mopf.go+4/-4
Mopf_test.go+2/-2
Mproviders.go+15/-6
Mredact_test.go+76/-5
4694 unmodified lines
4695
4696
4697
4698
4698
4699
4700
4701
4694 unmodified lines
// Summary.Intent and ReviewPrompt that previously bypassed redaction because
// the dispatcher only matched .jsonl. The PR 1236 fix extended the JSON-aware
// branch to .json. We assert via a low-entropy AWS-key shaped secret (catches
// the 7-layer pipeline) so the test stays deterministic without the OPF binary.
// the regex-only pipeline) so the test stays deterministic without the OPF binary.
func TestRedactBlobBytes_JSONMetadata(t *testing.T) {
t.Parallel()
Mcmd/entire/cli/checkpoint/checkpoint_test.go+1/-1
447 unmodified lines
448
449
450
451
451
452
453
454
260 unmodified lines
715
716
717
718
718
719
720
721
1532 unmodified lines
2254
2255
2256
2257
2258
2257
2258
2259
2260
2261
2261
2262
2262
2263
2264
2265
2266
15 unmodified lines
2282
2283
2284
2284
2285
2286
2287
2288
2289
2285
2286
2287
2288
2289
2290
2291
2292
2293
2294
35 unmodified lines
2330
2331
2332
2331
2332
2333
2334
2335
2336
2337
447 unmodified lines
}
}
// Replace prompts with 7-layer-redacted content.
// Replace prompts with regex-only-redacted content.
if len(opts.Prompts) > 0 {
promptContent := RedactedJoinedPrompts(opts.Prompts)
blobHash, err := CreateBlobFromContent(s.repo, []byte(promptContent))
260 unmodified lines
}
filePaths.AssetsManifest = manifestPath
// Write prompts via the 7-layer pipeline. OPF runs only in the
// Write prompts via the regex-only pipeline. OPF runs only in the
// pre-push rewrite path (manual_commit_opf_rewrite.go).
if len(opts.Prompts) > 0 {
promptContent := RedactedJoinedPrompts(opts.Prompts)
1532 unmodified lines
return fmt.Errorf("path traversal detected: %s", relPath)
}
// Create blob from file with 7-layer secrets redaction.
// Post-commit emits 7-layer-only blobs; the pre-push rewrite
// Create blob from file with regex-only secrets redaction (the
// eight always-on/opt-in layers).
// Post-commit emits regex-only blobs; the pre-push rewrite
// (strategy/manual_commit_opf_rewrite.go) walks the resulting
// tree, re-redacts these blobs with OPF when enabled, and
// rewrites entire/checkpoints/v1 into 8-layer commits before
// they leave the local machine.
// rewrites entire/checkpoints/v1 into OPF-applied (9-layer)
// commits before they leave the local machine.
blobHash, mode, err := createRedactedBlobFromFile(ctx, s.repo, path, relPath)
if err != nil {
return fmt.Errorf("failed to create blob for %s: %w", path, err)
15 unmodified lines
return nil
}
// createRedactedBlobFromFile reads a file, applies the 7-layer redaction
// pipeline, and creates a git blob. Used by committed-checkpoint writes
// at post-commit time. The OpenAI Privacy Filter is intentionally NOT
// run here — OPF lives in the pre-push rewrite path
// (strategy/manual_commit_opf_rewrite.go), which re-redacts the 7-layer
// blobs into 8-layer commits before they leave the local machine.
// createRedactedBlobFromFile reads a file, applies the regex-only redaction
// pipeline (the eight always-on/opt-in layers), and creates a git blob. Used
// by committed-checkpoint writes at post-commit time. The OpenAI Privacy
// Filter is intentionally NOT run here — OPF lives in the pre-push rewrite
// path (strategy/manual_commit_opf_rewrite.go), which re-redacts the
// regex-only blobs into OPF-applied (9-layer) commits before they leave the
// local machine.
// JSONL files get JSONL-aware redaction; all other files get plain byte redaction.
func createRedactedBlobFromFile(ctx context.Context, repo *git.Repository, filePath, treePath string) (plumbing.Hash, filemode.FileMode, error) {
info, err := os.Stat(filePath)
35 unmodified lines
// JSON-shaped files (.jsonl or .json) get JSON-aware redaction (falling
// back to plain bytes on parse failure so regex/credential layers
// still apply); other files get plain byte redaction. When
// usePrivacyFilter is true the full 8-layer pipeline (including OPF)
// runs; otherwise the 7-layer pipeline.
// usePrivacyFilter is true the full 9-layer pipeline (the eight regex
// layers plus OPF) runs; otherwise just the eight regex layers.
//
// .json is handled alongside .jsonl because checkpoint metadata files
// (metadata.json, per-session metadata.json) carry free-form fields
Mcmd/entire/cli/checkpoint/persistent.go+16/-14
17 unmodified lines
18
19
20
21
21
22
23
24
25
26
25
26
27
28
29
17 unmodified lines
// TestWriteCommitted_DoesNotEmitOPFAppliedTrailer is the regression guard
// for the architectural promise: standard post-commit condensation writes
// 7-layer-only blobs and MUST NOT mark them with the Entire-OPF-Applied
// regex-only blobs and MUST NOT mark them with the Entire-OPF-Applied
// trailer. The trailer is emitted exclusively by the pre-push rewrite
// path; if a future change accidentally added it to the standard writer,
// the pre-push rewrite would skip those commits (HasOPFApplied true →
// reparent-only, no actual OPF run) and ship 7-layer content as if it
// were 8-layer. This test pins down that contract.
// reparent-only, no actual OPF run) and ship regex-only content as if it
// were OPF-applied. This test pins down that contract.
func TestWriteCommitted_DoesNotEmitOPFAppliedTrailer(t *testing.T) {
t.Parallel()
Mcmd/entire/cli/checkpoint/persistent_opf_trailer_test.go+3/-3
22 unmodified lines
23
24
25
26
27
26
27
28
29
30
31
22 unmodified lines
return prompts
}
// RedactedJoinedPrompts joins prompts and runs the 7-layer redaction
// pipeline. OPF runs exclusively in the pre-push rewrite (not here),
// RedactedJoinedPrompts joins prompts and runs the regex-only redaction
// pipeline (the eight always-on/opt-in layers). OPF runs exclusively in
// the pre-push rewrite (not here),
// so the writer's hot path stays predictable. Exported so alternate
// persistent backends produce identically-redacted prompt blobs.
func RedactedJoinedPrompts(prompts []string) string {
Mcmd/entire/cli/checkpoint/prompts.go+3/-2
27 unmodified lines
28
29
30
31
32
31
32
33
34
35
36
27 unmodified lines
}
// TestRedactedJoinedPrompts_AppliesSafetyNet verifies the helper joins
// prompts with the canonical separator and runs them through the 7-layer
// pipeline. OPF runs only in the pre-push rewrite path, never here.
// prompts with the canonical separator and runs them through the
// regex-only pipeline. OPF runs only in the pre-push rewrite path, never
// here.
func TestRedactedJoinedPrompts_AppliesSafetyNet(t *testing.T) {
t.Parallel()
got := RedactedJoinedPrompts([]string{"hello", "world"})
Mcmd/entire/cli/checkpoint/prompts_test.go+3/-2
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), "\n") {
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()
32 unmodified lines
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\nnext\n")
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()
1 unmodified line
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))
}
// TestGitRemoteSectionExists_ExactSubsectionMatch verifies the check compares
// the whole URL subsection, not a prefix: a longer URL that shares a prefix
// (e.g. ".../repo.git") must not make a shorter one (".../repo") look present.
func TestGitRemoteSectionExists_ExactSubsectionMatch(t *testing.T) {
t.Parallel()
ctx := context.Background()
repoDir := t.TempDir()
testutil.InitRepo(t, repoDir)
const longURL = "https://example.com/org/repo.git"
const shortURL = "https://example.com/org/repo"
runIsolatedGit(ctx, t, repoDir, "config", "--local", "remote."+longURL+".promisor", "true")
assert.True(t, gitRemoteSectionExists(ctx, repoDir, longURL),
"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/checkpoint/remote/git_test.go+184/-19
312 unmodified lines
313
314
315
316
316
317
318
319
320
321
322
323
324
325
323
324
325
326
327
312 unmodified lines
var gitConfigGuardRepositoryFormatVersionRE = regexp.MustCompile(`(?m)^([ \t]*)repositoryformatversion = [01]$`)
var gitConfigGuardTransportPromisorRemoteRE = regexp.MustCompile(
`(?m)^\[remote "(?:(?:https?|ssh|file)://|/|[A-Za-z]:[\\/]|[^"\n]+@[^"\n]+:[^"\n]+).+"\]\n(?:[ \t]+(?:promisor = true|partialclonefilter = blob:none|skipFetchAll = true|skipDefaultUpdate = true)\n?){2,4}`,
`(?m)^\[remote "(?:(?:https?|ssh|file)://|/|[A-Za-z]:[\\/]|[^"\n]+@[^"\n]+:[^"\n]+).+"\]\n(?:[ \t]+(?:promisor = true|partialclonefilter = blob:none|skipFetchAll = true)\n?){2,3}`,
)
func normalizeGitConfigForGuard(content string) string {
content = gitConfigGuardRepositoryFormatVersionRE.ReplaceAllString(content, `${1}repositoryformatversion = <normalized>`)
// Deliberately ignore only the URL-keyed remote sections written during
// filtered fetches: git's promisor+partialclonefilter pair plus the
// skipFetchAll/skipDefaultUpdate stamp the CLI adds so bulk fetches skip
// the entry. A section without the full promisor pair (or with any other
// key) still fails loudly.
// skipFetchAll stamp the CLI adds so bulk fetches skip the entry. A section
// without the full promisor pair (or with any other key) still fails loudly.
content = gitConfigGuardTransportPromisorRemoteRE.ReplaceAllStringFunc(content, func(section string) string {
if strings.Contains(section, "promisor = true") && strings.Contains(section, "partialclonefilter = blob:none") {
return ""
Mcmd/entire/cli/integration_test/testenv.go+3/-4
79 unmodified lines
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
79 unmodified lines
os.Getenv("GIT_TERMINAL_PROMPT") == "0"
}
// IsTerminalReader reports whether r is an *os.File backed by a terminal.
// It is useful when an explicitly interactive command needs to distinguish a
// human at stdin from an agent process that merely inherited a controlling TTY.
func IsTerminalReader(r io.Reader) bool {
f, ok := r.(*os.File)
if !ok {
return false
}
return term.IsTerminal(int(f.Fd())) //nolint:gosec // G115: uintptr->int is safe for fd
}
// IsTerminalWriter reports whether w is an *os.File backed by a terminal.
// Use for deciding on color, pager, progress bars, or other writer-scoped
// TTY formatting. For "can I prompt the user?" use CanPromptInteractively.
Mcmd/entire/cli/interactive/interactive.go+11
209 unmodified lines
210
211
212
213
213
214
215
216
53 unmodified lines
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
53 unmodified lines
364
365
366
332
367
368
369
370
419 unmodified lines
790
791
792
758
793
794
795
796
22 unmodified lines
819
820
821
787
822
823
824
825
151 unmodified lines
977
978
979
945
980
981
982
983
984
950
985
986
987
988
55 unmodified lines
1044
1045
1046
1012
1047
1048
1049
1050
1051
47 unmodified lines
1099
1100
1101
1066
1102
1103
1069
1104
1105
1106
1107
80 unmodified lines
1188
1189
1190
1156
1191
1192
1193
1194
1195
75 unmodified lines
1271
1272
1273
1238
1274
1275
1276
1277
67 unmodified lines
1345
1346
1347
1312
1313
1314
1315
1316
1348
1349
1350
1351
1352
1353
1354
209 unmodified lines
}, deps)
}
if edit {
if !interactive.IsTerminalWriter(cmd.OutOrStdout()) || !interactive.CanPromptInteractively() {
if !reviewCommandIsInteractive(cmd) {
err := errors.New("--edit requires an interactive terminal")
cmd.SilenceUsage = true
fmt.Fprintln(cmd.ErrOrStderr(), "--edit requires an interactive terminal.")
53 unmodified lines
Slots []string // reviewer slots as "agent[=model]" entries (--set-slot)
}
// reviewCommandIsInteractive requires the exact stdin consumed by huh and
// Bubble Tea, plus stdout, to be terminals. CanPromptInteractively adds the
// independent policy gate for tests, CI, and agent subprocess sentinels; a
// controlling /dev/tty alone is insufficient because stdin may still be piped.
func reviewCommandIsInteractive(cmd *cobra.Command) bool {
hardDisabled := reviewInteractivityHardDisabled(
os.Getenv(interactive.EnvTestTTY),
os.Getenv("CI"),
interactive.UnderTest(),
)
return reviewTTYIsInteractive(
interactive.IsTerminalReader(cmd.InOrStdin()),
interactive.IsTerminalWriter(cmd.OutOrStdout()),
interactive.CanPromptInteractively(),
hardDisabled,
)
}
func reviewInteractivityHardDisabled(testTTY, ci string, underTest bool) bool {
// Match CanPromptInteractively's precedence: ENTIRE_TEST_TTY=1 may opt an
// in-process test into interaction, while tests without that explicit
// override must never read from a developer's real terminal.
if testTTY != "" {
return testTTY != "1"
}
return underTest || (ci != "" && ci != "false")
}
func reviewTTYIsInteractive(stdinTTY, stdoutTTY, canPrompt, hardDisabled bool) bool {
// Real stdio terminals are necessary but not sufficient: agent shells can
// allocate a PTY while advertising that no human is available through the
// sentinels enforced by CanPromptInteractively.
return !hardDisabled && stdinTTY && stdoutTTY && canPrompt
}
func (o reviewConfigureOptions) scripted() bool {
// Local selects the destination only; by itself it must not force the
// non-interactive/scripted path. `entire review --configure --local` should
53 unmodified lines
// duplicate the catalog here. Pass the raw --profile value (empty when not
// given) so the guided setup runs the "what kind of review?" type picker
// instead of being silently defaulted to the general profile.
if interactive.IsTerminalWriter(out) && interactive.CanPromptInteractively() {
if reviewCommandIsInteractive(cmd) {
name, profile, setupErr := RunReviewGuidedSetup(ctx, out, installed, deps.ReviewerFor, strings.TrimSpace(profileOverride), false, s)
if setupErr != nil {
return handlePickerError(cmd, silentErr, setupErr)
419 unmodified lines
applyLegacyReviewProfileFallback(s)
profileOverride = strings.TrimSpace(profileOverride)
interactiveTTY := interactive.IsTerminalWriter(out) && interactive.CanPromptInteractively()
interactiveTTY := reviewCommandIsInteractive(cmd)
// Bare `entire review` never auto-runs a profile. Without a TTY we cannot
// prompt, so list the profiles (or point at setup) and require an explicit
22 unmodified lines
// Non-interactive first run writes the shared project settings; interactive
// setup asks the user where to save below.
saveScope := reviewScopeProject
guidedSetup := interactive.IsTerminalWriter(out) && interactive.CanPromptInteractively()
guidedSetup := interactiveTTY
if guidedSetup {
var setupErr error
profileForSetup, profile, setupErr = RunReviewGuidedSetup(ctx, out, installed, deps.ReviewerFor, profileForSetup, true, s)
151 unmodified lines
// (true, nil). In a non-interactive context it cannot prompt, so it proceeds
// (the user explicitly invoked `entire review`) after printing a note rather
// than blocking on a confirm form that would error out.
func confirmReReviewOrProceed(ctx context.Context, out io.Writer, deps Deps) (bool, error) {
func confirmReReviewOrProceed(ctx context.Context, out io.Writer, deps Deps, canPrompt bool) (bool, error) {
reviewed, meta := deps.HeadHasReviewCheckpoint(ctx)
if !reviewed {
return true, nil
}
if !interactive.CanPromptInteractively() {
if !canPrompt {
fmt.Fprintf(out, "Note: HEAD was already reviewed (%s); re-running.\n", meta)
return true, nil
}
55 unmodified lines
}
// 4. Re-run guard: check if HEAD's checkpoint already has a review.
if proceed, guardErr := confirmReReviewOrProceed(ctx, out, deps); guardErr != nil {
canPrompt := reviewCommandIsInteractive(cmd)
if proceed, guardErr := confirmReReviewOrProceed(ctx, out, deps, canPrompt); guardErr != nil {
fmt.Fprintln(out, "prompt cancelled")
return silentErr(guardErr)
} else if !proceed {
47 unmodified lines
defer cancelRun()
runCfg.EnrichSummary = reviewSummaryTokenEnricher(worktreeRoot, headSHA)
canPrompt := interactive.CanPromptInteractively()
sinks := composeSingleAgentSinks(singleAgentSinkInputs{
out: out,
isTTY: interactive.IsTerminalWriter(out) && canPrompt,
isTTY: canPrompt,
canPrompt: canPrompt,
agentName: displayName,
cancelRun: cancelRun,
80 unmodified lines
return fmt.Errorf("resolve HEAD: %w", shaErr)
}
if proceed, guardErr := confirmReReviewOrProceed(ctx, out, deps); guardErr != nil {
canPrompt := reviewCommandIsInteractive(cmd)
if proceed, guardErr := confirmReReviewOrProceed(ctx, out, deps, canPrompt); guardErr != nil {
fmt.Fprintln(out, "prompt cancelled")
return deps.NewSilentError(guardErr)
} else if !proceed {
75 unmodified lines
masterLabel := judgeLabel(judge)
sinks := composeMultiAgentSinks(multiAgentSinkInputs{
out: out,
isTTY: interactive.IsTerminalWriter(out) && interactive.CanPromptInteractively(),
isTTY: canPrompt,
agentNames: agentNames,
cancelRun: cancelRun,
runContext: runCtx,
67 unmodified lines
// instead of monkey-patching interactive helpers at run time.
//
// isTTY here means "the TUI sink is safe to compose" — production callers
// AND IsTerminalWriter(out) with CanPromptInteractively() before passing
// it in, since the TUI both writes ANSI to stdout AND reads keypresses
// from stdin. A terminal-stdout-but-non-interactive-stdin scenario (an
// agent host like Claude Code invoking `entire review`) must NOT use the
// TUI — its dismissal loop would block forever.
// use reviewCommandIsInteractive before passing it in, since the TUI both
// writes ANSI to stdout and reads keypresses from stdin. A terminal stdout
// with non-interactive stdin must not use the TUI; its dismissal loop would
// block forever.
type multiAgentSinkInputs struct {
out io.Writer
isTTY bool
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{"/review"}, // stale legacy entry
Skills: []string{"$missing-review"},
},
}); err != nil {
t.Fatal(err)
37 unmodified lines
t.Error("codex reviewer started despite failing skill validation")
}
stderr := errBuf.String()
if !strings.Contains(stderr, "/review") || !strings.Contains(stderr, "skipping") {
if !strings.Contains(stderr, "$missing-review") || !strings.Contains(stderr, "skipping") {
t.Errorf("stderr should warn about the excluded worker and its skill; got:\n%s", stderr)
}
}
6 unmodified lines
t.Setenv("HOME", t.TempDir())
if err := seedReviewConfig(context.Background(), map[string]settings.ReviewConfig{
testCodexAgent: {Skills: []string{"/review"}},
testCodexAgent: {Skills: []string{"$missing-review"}},
"gemini": {Skills: []string{"$also-missing"}},
}); err != nil {
t.Fatal(err)
Mcmd/entire/cli/review/cmd_test.go+70/-10
42 unmodified lines
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
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
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
42 unmodified lines
}
}
func TestDefaultReviewAgentConfig_CodexIsPromptOnly(t *testing.T) {
t.Parallel()
cfg := defaultReviewAgentConfig(DefaultProfileName, tAgentCodex)
if len(cfg.Skills) != 0 {
t.Fatalf("Codex default skills = %v, want none", cfg.Skills)
}
if cfg.Prompt != defaultAgentReviewPrompt {
t.Fatalf("Codex default prompt = %q, want %q", cfg.Prompt, defaultAgentReviewPrompt)
}
}
func TestApplyLegacyReviewProfileFallback_RepairsGeneratedCodexSkill(t *testing.T) {
t.Parallel()
s := &settings.EntireSettings{ReviewProfiles: map[string]settings.ReviewProfileConfig{
DefaultProfileName: {Agents: map[string]settings.ReviewConfig{
tAgentCodex: {Skills: []string{"/review"}},
"codex-opus": {
Agent: tAgentCodex,
Model: "o3",
Skills: []string{"/review"},
},
"codex-custom": {
Agent: tAgentCodex,
Skills: []string{"$security-audit"},
},
}},
}}
applyLegacyReviewProfileFallback(s)
got := s.ReviewProfiles[DefaultProfileName].Agents[tAgentCodex]
if len(got.Skills) != 0 || got.Prompt != defaultAgentReviewPrompt {
t.Fatalf("repaired Codex config = %+v, want prompt-only default", got)
}
alias := s.ReviewProfiles[DefaultProfileName].Agents["codex-opus"]
if len(alias.Skills) != 0 || alias.Prompt != defaultAgentReviewPrompt || alias.Model != "o3" {
t.Fatalf("repaired aliased Codex config = %+v", alias)
}
custom := s.ReviewProfiles[DefaultProfileName].Agents["codex-custom"]
if len(custom.Skills) != 1 || custom.Skills[0] != "$security-audit" {
t.Fatalf("custom Codex config changed: %+v", custom)
}
}
func TestConfirmReReviewOrProceed_NonInteractiveDoesNotPrompt(t *testing.T) {
t.Parallel()
out := &bytes.Buffer{}
proceed, err := confirmReReviewOrProceed(context.Background(), out, Deps{
HeadHasReviewCheckpoint: func(context.Context) (bool, string) {
return true, "existing review"
},
}, false)
if err != nil {
t.Fatalf("confirmReReviewOrProceed: %v", err)
}
if !proceed {
t.Fatal("non-interactive re-review should proceed")
}
if !strings.Contains(out.String(), "already reviewed") {
t.Fatalf("missing non-interactive re-review note: %q", out.String())
}
}
func TestReviewInteractivityHardDisabled(t *testing.T) {
t.Parallel()
tests := []struct {
name string
testTTY string
ci string
underTest bool
want bool
}{
{name: "go test defaults off", underTest: true, want: true},
{name: "test override enables", testTTY: "1", ci: "true", underTest: true, want: false},
{name: "test override disables", testTTY: "0", want: true},
{name: "CI disables", ci: "true", want: true},
{name: "CI false does not disable", ci: "false", want: false},
{name: "normal process", want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
if got := reviewInteractivityHardDisabled(tt.testTTY, tt.ci, tt.underTest); got != tt.want {
t.Fatalf("reviewInteractivityHardDisabled(%q, %q, %v) = %v, want %v", tt.testTTY, tt.ci, tt.underTest, got, tt.want)
}
})
}
}
func TestReviewTTYIsInteractive(t *testing.T) {
t.Parallel()
tests := []struct {
name string
stdinTTY bool
stdoutTTY bool
canPrompt bool
hardDisabled bool
want bool
}{
{name: "direct human terminal", stdinTTY: true, stdoutTTY: true, canPrompt: true, want: true},
{name: "agent sentinel overrides real PTY", stdinTTY: true, stdoutTTY: true, canPrompt: false, want: false},
{name: "controlling terminal does not override piped stdin", stdinTTY: false, stdoutTTY: true, canPrompt: true, want: false},
{name: "captured stdout", stdinTTY: true, stdoutTTY: false, canPrompt: true, want: false},
{name: "agent with piped stdin", stdinTTY: false, stdoutTTY: true, canPrompt: false, want: false},
{name: "explicitly forced non-interactive", stdinTTY: true, stdoutTTY: true, canPrompt: true, hardDisabled: true, want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
if got := reviewTTYIsInteractive(tt.stdinTTY, tt.stdoutTTY, tt.canPrompt, tt.hardDisabled); got != tt.want {
t.Fatalf("reviewTTYIsInteractive(%v, %v, %v, %v) = %v, want %v", tt.stdinTTY, tt.stdoutTTY, tt.canPrompt, tt.hardDisabled, got, tt.want)
}
})
}
}
func TestBuildConfiguredProfile_FromFlags(t *testing.T) {
t.Parallel()
deps := configureTestDeps("claude-code", "codex")
Mcmd/entire/cli/review/configure_test.go+120
11 unmodified lines
12
13
14
15
15
16
17
28 unmodified lines
46
47
48
50
49
50
51
52
11 unmodified lines
"charm.land/huh/v2"
"github.com/spf13/cobra"
"github.com/entireio/cli/cmd/entire/cli/interactive"
"github.com/entireio/cli/cmd/entire/cli/mdrender"
"github.com/entireio/cli/cmd/entire/cli/paths"
"github.com/entireio/cli/cmd/entire/cli/stringutil"
28 unmodified lines
fmt.Fprintln(cmd.OutOrStdout(), "No local review findings found.")
return nil
}
if interactive.IsTerminalWriter(cmd.OutOrStdout()) && interactive.CanPromptInteractively() {
if reviewCommandIsInteractive(cmd) {
manifest, pickErr := promptForReviewManifest(ctx, manifests)
if pickErr != nil {
return pickErr
Mcmd/entire/cli/review/fix.go+1/-2
115 unmodified lines
116
117
118
119
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
10 unmodified lines
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
200 unmodified lines
365
366
367
368
369
370
371
372
2 unmodified lines
375
376
377
350
351
352
353
378
379
380
381
382
115 unmodified lines
}
func applyLegacyReviewProfileFallback(s *settings.EntireSettings) {
if s == nil || len(nonZeroProfiles(s.ReviewProfiles)) > 0 {
if s == nil {
return
}
// Older guided setup wrote Codex reviewers with Claude's curated /review
// command. Codex has no such built-in, so spawn-time validation excludes
// those workers. Repair that generated shape in memory to a prompt-only
// Codex reviewer; explicitly configured Codex skills are left untouched.
normalizeLegacyCodexDefaultSkills(s.Review) //nolint:staticcheck // intentional compatibility repair for deprecated review config
for name, profile := range s.ReviewProfiles {
normalizeLegacyCodexDefaultSkills(profile.Agents)
s.ReviewProfiles[name] = profile
}
if len(nonZeroProfiles(s.ReviewProfiles)) > 0 {
return
}
legacyAgents := nonZeroAgentConfigs(s.Review) //nolint:staticcheck // intentional compatibility fallback for deprecated review config
10 unmodified lines
}
}
func normalizeLegacyCodexDefaultSkills(configs map[string]settings.ReviewConfig) {
for workerName, cfg := range configs {
if reviewAgentName(workerName, cfg) != string(agent.AgentNameCodex) ||
len(cfg.Skills) != 1 || strings.TrimSpace(cfg.Skills[0]) != "/review" {
continue
}
cfg.Skills = nil
if strings.TrimSpace(cfg.Prompt) == "" {
cfg.Prompt = defaultAgentReviewPrompt
}
configs[workerName] = cfg
}
}
func nonZeroProfiles(in map[string]settings.ReviewProfileConfig) map[string]settings.ReviewProfileConfig {
return nonZeroNamed(in)
}
200 unmodified lines
return profile, nil
}
const defaultAgentReviewPrompt = "Review the change according to the profile task."
func defaultReviewAgentConfig(profileName, agentName string) settings.ReviewConfig {
focus := defaultProfileFocus(profileName)
switch agentName {
2 unmodified lines
return settings.ReviewConfig{Skills: []string{"/security-review"}}
}
return settings.ReviewConfig{Skills: []string{"/review"}, Prompt: focus}
case string(agent.AgentNameCodex):
return settings.ReviewConfig{Skills: []string{"/review"}, Prompt: focus}
case string(agent.AgentNameGemini), string(agent.AgentNamePi):
prompt := "Review the change according to the profile task."
case string(agent.AgentNameCodex), string(agent.AgentNameGemini), string(agent.AgentNamePi):
prompt := defaultAgentReviewPrompt
if focus != "" {
prompt += " " + focus
}
Mcmd/entire/cli/review/profile.go+31/-5
283 unmodified lines
284
285
286
287
287
288
289
290
283 unmodified lines
// PromptDefault controls whether the pre-push hook asks the user
// before running OPF. "" (default) and "ask" both surface the
// interactive prompt; "never" skips OPF and pushes 7-layer content;
// interactive prompt; "never" skips OPF and pushes regex-only content;
// "always" runs without asking. ENTIRE_OPF=yes|no on the push
// invocation overrides this setting per-push.
PromptDefault string `json:"prompt_default,omitempty"`
Mcmd/entire/cli/settings/settings.go+1/-1
441 unmodified lines
442
443
444
445
445
446
447
448
441 unmodified lines
})
}
// OpenAI Privacy Filter (opt-in 8th layer).
// OpenAI Privacy Filter (opt-in 9th layer).
if s.Redaction != nil && s.Redaction.OpenAIPrivacyFilter != nil {
opf := s.Redaction.OpenAIPrivacyFilter
redact.ConfigurePrivacyFilter(redact.OPFConfig{
Mcmd/entire/cli/strategy/common.go+1/-1
104 unmodified lines
105
106
107
108
109
110
108
109
110
111
112
112
113
114
115
116
117
239 unmodified lines
357
358
359
358
360
361
362
363
104 unmodified lines
allAgentFiles map[string]struct{} // Union of all sessions' FilesTouched for cross-session exclusion (nil = single-session)
}
// redactSessionJSONLBytes runs the 7-layer redaction pipeline over a
// session transcript at post-commit condensation. OPF is intentionally
// NOT included here — it runs exclusively in the pre-push rewrite path
// redactSessionJSONLBytes runs the regex-only redaction pipeline (the
// eight always-on/opt-in layers) over a session transcript at
// post-commit condensation. OPF is intentionally NOT included here —
// it runs exclusively in the pre-push rewrite path
// (strategy/manual_commit_opf_rewrite.go), which re-redacts the
// 7-layer blobs and produces 8-layer commits before the push.
// regex-only blobs and produces OPF-applied (9-layer) commits before
// the push.
//
// Exposed as a var so tests can inject deterministic success/error
// returns. The signature still takes a context so the var can be
239 unmodified lines
summary = generateSummary(ctx, redactedTranscript, sessionData.FilesTouched, state)
}
// Post-commit emits 7-layer-only blobs. OPF runs later in the
// Post-commit emits regex-only blobs. OPF runs later in the
// pre-push rewrite path, never here.
skillEvents := mergeSkillEvents(state.SkillEvents, withSkillEventTurnID(sessionData.SkillEvents, state.TurnID))
Mcmd/entire/cli/strategy/manual_commit_condensation.go+7/-5
2841 unmodified lines
2842
2843
2844
2845
2846
2847
2845
2846
2847
2848
2849
2850
44 unmodified lines
2895
2896
2897
2898
2898
2899
2900
2901
2841 unmodified lines
// (attribution, files touched, prompts). Hooks run without user interaction
// so there is no retry path — preserving partial metadata is better than
// losing everything. Persisting an unredacted transcript would be worse.
// Run the 7-layer pipeline over the transcript — OPF runs later in
// the pre-push rewrite path, which re-redacts these 7-layer blobs
// and produces 8-layer commits before the push goes out.
// Run the regex-only pipeline over the transcript — OPF runs later in
// the pre-push rewrite path, which re-redacts these regex-only blobs
// and produces OPF-applied (9-layer) commits before the push goes out.
// Externalize inline images BEFORE redaction, mirroring CondenseSession, so the
// finalized (authoritative, full-session) transcript keeps its placeholders and
// matching assets instead of re-inlining what condensation lifted out. Opt-in;
44 unmodified lines
redactedTranscript = redact.RedactedBytes{}
}
// Post-commit emits 7-layer-only blobs; the writer joins + redacts
// Post-commit emits regex-only blobs; the writer joins + redacts
// via checkpoint.redactedJoinedPrompts. OPF runs later, once per
// push, in the pre-push rewrite path.
stores, err := checkpoint.Open(ctx, repo, checkpoint.OpenOptions{})
Mcmd/entire/cli/strategy/manual_commit_hooks.go+4/-4
20 unmodified lines
21
22
23
24
25
24
25
26
27
28
20 unmodified lines
type OPFDecision int
const (
OPFRun OPFDecision = iota // run the rewrite, push 8-layer
OPFSkip // skip the rewrite, push 7-layer
OPFRun OPFDecision = iota // run the rewrite, push OPF-applied (9-layer)
OPFSkip // skip the rewrite, push regex-only (8-layer)
OPFAbort // cancel the push entirely (Ctrl-C / non-TTY abort)
)
Mcmd/entire/cli/strategy/manual_commit_opf_prompt.go+2/-2
1
2
3
4
5
6
4
5
6
7
8
9
70 unmodified lines
80
81
82
83
84
85
86
83
84
85
86
87
88
89
90
91
92
93
93
94
95
96
158 unmodified lines
255
256
257
258
258
259
260
261
31 unmodified lines
293
294
295
296
296
297
298
299
187 unmodified lines
487
488
489
490
490
491
492
493
// Pre-push OPF rewrite for entire/checkpoints/v1.
//
// This is the ONLY production code path that runs the OPF-augmented
// redaction entry points. Post-commit condensation stays on 7-layer
// for predictable latency; OPF runs here, once per push, after the
// user opted in via settings.
// redaction entry points. Post-commit condensation stays on the
// regex-only pipeline for predictable latency; OPF runs here, once per
// push, after the user opted in via settings.
package strategy
import (
70 unmodified lines
}
// OPFRuntimeFailedError: the OPF circuit breaker tripped mid-rewrite.
// Some blobs were silently downgraded to 7-layer; tagging those commits
// as Entire-OPF-Applied would be a privacy regression (future pushes
// would skip them while their content is 7-layer-only). Abort before
// CAS so the user fixes their OPF install and retries.
// Some blobs were silently downgraded to regex-only; tagging those
// commits as Entire-OPF-Applied would be a privacy regression (future
// pushes would skip them while their content is regex-only). Abort
// before CAS so the user fixes their OPF install and retries.
type OPFRuntimeFailedError struct {
OPFCommand string
}
func (e *OPFRuntimeFailedError) Error() string {
return fmt.Sprintf("OPF runtime failed during pre-push rewrite (command=%q); "+
"aborting push so 7-layer content isn't tagged as 8-layer-applied. "+
"aborting push so regex-only content isn't tagged as OPF-applied. "+
"Run `%s --help` to verify your OPF install, then retry. Or set "+
"ENTIRE_OPF=no on the push to skip OPF for this push only.",
e.OPFCommand, e.OPFCommand)
158 unmodified lines
// this push (an earlier process step tripped the breaker), abort
// before tagging any commits as OPF-applied. Without this, the
// per-blob fallback inside the no-OPF cases of BatchBytesWithPrivacyFilter
// could let 7-layer content slip out with the trailer attached.
// could let regex-only content slip out with the trailer attached.
if redact.OPFBreakerTripped() {
return plumbing.ZeroHash, &OPFRuntimeFailedError{OPFCommand: redact.OPFCommand()}
}
31 unmodified lines
}
pc.startIdx = len(globalBlobs)
// Whole-tree redaction: each v1 commit tree is cumulative, so the
// newest commit can still carry older shards that were 7-layer-only
// newest commit can still carry older shards that were regex-only
// before this rewrite. Redacting the whole tree for every unapplied
// commit keeps the final rewritten tip from reintroducing an
// un-OPF-redacted older shard. collect and apply walk the tree the
187 unmodified lines
//
// Correctness note: each v1 commit tree is cumulative. During a multi-commit
// rewrite, the newest original commit can still contain older shards that were
// 7-layer-only before this rewrite. The collect/apply walkers redact the whole
// regex-only before this rewrite. The collect/apply walkers redact the whole
// tree for every unapplied commit so the final rewritten tip cannot
// reintroduce an older un-OPF-redacted shard.
func rebuildV1Commit(ctx context.Context, repo *git.Repository, oldCommit *object.Commit, parent plumbing.Hash, redactedByPath map[string][]byte) (plumbing.Hash, error) {
Mcmd/entire/cli/strategy/manual_commit_opf_rewrite.go+11/-11
655 unmodified lines
656
657
658
659
660
659
660
661
662
663
655 unmodified lines
// Fail-closed regression: when the OPF runtime fails and the breaker
// trips, the rewrite must NOT CAS the ref. Otherwise the new commits
// would carry Entire-OPF-Applied: true while their content is 7-layer
// only, and future pushes would skip them — silently shipping unredacted
// would carry Entire-OPF-Applied: true while their content is regex-only,
// and future pushes would skip them — silently shipping unredacted
// content to the remote.
func TestRewriteUnpushedV1WithOPF_BreakerTrippedMidRewrite_AbortsBeforeCAS(t *testing.T) {
configureFakeOPF(t, &fakeRuntimeAlwaysFails{})
Mcmd/entire/cli/strategy/manual_commit_opf_rewrite_test.go+2/-2
70 unmodified lines
71
72
73
74
75
76
74
75
76
77
78
79
12 unmodified lines
92
93
94
95
95
96
97
98
70 unmodified lines
// OPF pre-push rewrite: if OPF is configured, resolve the user's
// decision (env > settings > prompt > non-TTY auto-run), then
// re-redact unpushed v1 commits with the 8-layer pipeline before
// pushing. Skipped entirely when OPF is off, so the common-case
// fast path is unchanged.
// re-redact unpushed v1 commits with OPF (producing the OPF-applied,
// 9-layer pipeline) before pushing. Skipped entirely when OPF is off,
// so the common-case fast path is unchanged.
if redact.OPFEnabled() {
cfg, _ := settings.Load(ctx) //nolint:errcheck // Load already failed at hook init; fall back to nil
var opfCfg *settings.OPFSettings
12 unmodified lines
return errOPFAbortedByUser
case OPFSkip:
// User opted out for this push (or settings/env say
// "never"). Push 7-layer content as-is.
// "never"). Push regex-only (8-layer) content as-is.
logging.Info(ctx, "OPF skipped for this push (user choice or settings)")
case OPFRun:
_, opfSpan := perf.Start(ctx, "opf_pre_push_rewrite")
Mcmd/entire/cli/strategy/manual_commit_push.go+4/-4
50 unmodified lines
51
52
53
54
54
55
56
57
58
50 unmodified lines
AgentTrailerKey = "Entire-Agent"
// OPFAppliedTrailerKey marks an entire/checkpoints/v1 commit whose blobs
// have been redacted by the OpenAI Privacy Filter (8-layer pipeline).
// have been redacted by the OpenAI Privacy Filter (the opt-in 9th,
// network-backed layer, applied on top of the 8 regex layers).
// Format: literal "true"; the trailer is omitted entirely when OPF was
// not applied. The pre-push rewrite path treats commits lacking this
// trailer as candidates to OPF-redact before they reach the remote.
Mcmd/entire/cli/trailers/trailers.go+2/-1
350 unmodified lines
351
352
353
354
355
354
355
356
357
358
359
350 unmodified lines
// TestHasOPFApplied covers the Entire-OPF-Applied trailer reader. The
// trailer marks a v1 commit whose blobs have been redacted by the
// OpenAI Privacy Filter (8-layer); commits without it carry 7-layer
// content and are eligible for the pre-push rewrite to add OPF.
// OpenAI Privacy Filter (OPF-applied, 9-layer); commits without it carry
// regex-only (8-layer) content and are eligible for the pre-push rewrite
// to add OPF.
func TestHasOPFApplied(t *testing.T) {
t.Parallel()
cases := []struct {
Mcmd/entire/cli/trailers/trailers_test.go+3/-2
27 unmodified lines
28
29
30
31
32
33
34
31
32
33
34
35
36
37
37
38
39
40
41
2 unmodified lines
44
45
46
46
47
48
49
50
50
51
52
53
54
60 unmodified lines
115
116
117
117
118
119
120
121
8 unmodified lines
130
131
132
132
133
134
135
136
9 unmodified lines
146
147
148
148
149
150
151
151
152
153
154
155
27 unmodified lines
// returns a non-nil error. Callers running this for privacy-critical
// operations (e.g. the pre-push rewrite) must abort rather than
// proceed with partially-redacted content. The per-blob
// JSONLContentWithPrivacyFilter falls back to 7-layer on batch
// failure; this batched variant intentionally does not, because the
// only caller (cross-blob walker) needs an explicit signal that OPF
// did not finish.
// JSONLContentWithPrivacyFilter falls back to the regex-only pipeline
// (the eight always-on/opt-in layers, no OPF) on batch failure; this
// batched variant intentionally does not, because the only caller
// (cross-blob walker) needs an explicit signal that OPF did not
// finish.
//
// When OPF is unconfigured, disabled, has no enabled categories, or
// the per-process circuit breaker has tripped, returns 7-layer-only
// the per-process circuit breaker has tripped, returns regex-only
// output for every blob with no error. This matches the existing
// non-batched paths and keeps the caller's hot-path code clean.
func BatchBytesWithPrivacyFilter(ctx context.Context, inputs []NamedBlob) ([][]byte, error) {
2 unmodified lines
}
cfg := getOPFConfig()
if cfg == nil || !cfg.Enabled || cfg.runtime == nil || opfBreakerTripped.Load() {
return apply7LayerToBlobs(inputs), nil
return applyRegexLayersToBlobs(inputs), nil
}
cats := enabledCategories(cfg)
if len(cats) == 0 {
return apply7LayerToBlobs(inputs), nil
return applyRegexLayersToBlobs(inputs), nil
}
// Pass 1: collect unique prose-shaped leaves across every blob.
60 unmodified lines
//
// JSON parse failures fall back to whole-content treatment, matching
// RedactBlobBytes's behavior: a malformed JSON blob still gets the
// 7-layer pipeline applied, just without leaf-by-leaf precision.
// regex-only pipeline applied, just without leaf-by-leaf precision.
func collectLeaves(in NamedBlob, add func(string)) {
if isJSONLikeName(in.Name) {
if _, err := jsonlContentImpl(string(in.Content), func(v string) string {
8 unmodified lines
}
// applyToBlob produces the redacted bytes for a single blob, combining
// the 7 regex layers with the cached OPF spans for each leaf. The
// the always-on/opt-in regex layers with the cached OPF spans for each leaf. The
// per-leaf closure mirrors JSONLContentWithPrivacyFilter's Pass 3.
func applyToBlob(in NamedBlob, spansByInput map[string][]Span, cfg *OPFConfig) []byte {
applier := func(v string) string {
9 unmodified lines
return []byte(applier(string(in.Content)))
}
// apply7LayerToBlobs is the OPF-disabled fast path: each blob gets
// applyRegexLayersToBlobs is the OPF-disabled fast path: each blob gets
// regex-only redaction with no shell-out. Returned slice is index-aligned
// with inputs.
func apply7LayerToBlobs(inputs []NamedBlob) [][]byte {
func applyRegexLayersToBlobs(inputs []NamedBlob) [][]byte {
out := make([][]byte, len(inputs))
for i, in := range inputs {
if isJSONLikeName(in.Name) {
Mredact/batch.go+12/-11
201 unmodified lines
202
203
204
205
205
206
207
208
68 unmodified lines
277
278
279
280
280
281
282
283
284
285
284
285
286
287
288
6 unmodified lines
295
296
297
298
298
299
300
301
302
302
303
304
305
306
307
307
308
309
310
201 unmodified lines
// TestBatchBytesWithPrivacyFilter_FailsClosedOnBatchError is the
// fail-closed contract: when the OPF runtime errors, callers must see
// the error rather than silently get 7-layer-only output tagged as if
// the error rather than silently get regex-only output tagged as if
// OPF ran. This is the privacy-critical difference vs
// JSONLContentWithPrivacyFilter (which silently falls back).
func TestBatchBytesWithPrivacyFilter_FailsClosedOnBatchError(t *testing.T) {
68 unmodified lines
}
}
// TestBatchBytesWithPrivacyFilter_OPFDisabledReturns7Layer covers the
// TestBatchBytesWithPrivacyFilter_OPFDisabledReturnsRegexOnly covers the
// "OPF turned off in settings" path: every blob gets regex-only
// redaction, no shell-out happens, no error. Without this, a user with
// OPF disabled would get a hard error from the new API instead of the
// fast 7-layer path they expect.
func TestBatchBytesWithPrivacyFilter_OPFDisabledReturns7Layer(t *testing.T) {
// fast regex-only path they expect.
func TestBatchBytesWithPrivacyFilter_OPFDisabledReturnsRegexOnly(t *testing.T) {
resetOPFConfig()
t.Cleanup(resetOPFConfig)
// No ConfigurePrivacyFilter call → cfg == nil
6 unmodified lines
t.Fatalf("OPF-disabled path should not error: %v", err)
}
if !strings.Contains(string(got[0]), "REDACTED") {
t.Errorf("7-layer fallback should still redact AWS key, got %q", string(got[0]))
t.Errorf("regex-only fallback should still redact AWS key, got %q", string(got[0]))
}
}
// TestBatchBytesWithPrivacyFilter_BreakerTrippedReturns7Layer ensures
// TestBatchBytesWithPrivacyFilter_BreakerTrippedReturnsRegexOnly ensures
// that once the circuit breaker has tripped (e.g. an earlier batch
// failed and the strategy aborted), subsequent calls in the same
// process don't pay another shell-out cost. They short-circuit to
// regex-only with no error.
func TestBatchBytesWithPrivacyFilter_BreakerTrippedReturns7Layer(t *testing.T) {
func TestBatchBytesWithPrivacyFilter_BreakerTrippedReturnsRegexOnly(t *testing.T) {
fake := &fakeRuntime{spans: []Span{{Start: 0, End: 5, Label: "private_person"}}}
configureFakeOPF(t, fake, map[string]bool{"private_person": true})
opfBreakerTripped.Store(true)
Mredact/batch_test.go+7/-7
87 unmodified lines
88
89
90
91
91
92
93
94
6 unmodified lines
101
102
103
104
104
105
106
106
107
108
109
75 unmodified lines
185
186
187
188
188
189
190
191
87 unmodified lines
// OPFEnabled reports whether the OpenAI Privacy Filter is configured
// and turned on for this process. Callers gate pre-push rewrite work
// on this: when false, the pre-push hook pushes the local 7-layer
// on this: when false, the pre-push hook pushes the local regex-only
// checkpoint branch verbatim with no extra processing. Independent of
// the circuit breaker — a tripped breaker still reports Enabled=true
// because the runtime config didn't change; the rewrite logic itself
6 unmodified lines
// OPFBreakerTripped reports whether the per-process OPF circuit breaker
// has been tripped — i.e. an OPF invocation failed at some point during
// this process's lifetime. The pre-push rewrite uses this to detect
// when OPF silently fell back to 7-layer mid-rewrite and abort before
// when OPF silently fell back to regex-only mid-rewrite and abort before
// CAS-ing the new ref; otherwise the rewritten commits would carry the
// Entire-OPF-Applied: true trailer despite containing only 7-layer
// Entire-OPF-Applied: true trailer despite containing only regex-only
// content, and the next push would skip them.
func OPFBreakerTripped() bool {
return opfBreakerTripped.Load()
75 unmodified lines
// in the pre-push rewrite path (strategy/manual_commit_opf_rewrite.go),
// whose hook is installed without a `2>/dev/null` redirect, so plain
// stderr reaches the user's terminal during `git push`. Post-commit
// condensation never invokes OPF (it calls the 7-layer functions
// condensation never invokes OPF (it calls the regex-layer functions
// directly via RedactBlobBytes(..., usePrivacyFilter=false)), so the
// historical `/dev/tty` routing that survived the post-commit hook's
// stderr redirect is no longer needed. Tests override this directly.
Mredact/opf.go+4/-4
607 unmodified lines
608
609
610
611
611
612
613
614
11 unmodified lines
626
627
628
629
629
630
631
632
607 unmodified lines
// TestJSONLContentWithPrivacyFilter_ShortReturnTripsBreaker pins the
// privacy contract: if the OPF runtime returns fewer span slices than
// inputs, we treat it as a runtime failure (trip the breaker + 7-layer
// inputs, we treat it as a runtime failure (trip the breaker + regex-only
// fallback) rather than silently produce under-redacted output. The
// per-blob caller in the pre-push rewrite then catches the tripped
// breaker via OPFBreakerTripped() and aborts before CAS.
11 unmodified lines
content := `{"a":"Alice met Bob","b":"Charlie sat down","c":"Eve walked home"}`
_, err := JSONLContentWithPrivacyFilter(context.Background(), content)
if err != nil {
t.Fatalf("short return should fall back to 7-layer (no error), got %v", err)
t.Fatalf("short return should fall back to regex-only (no error), got %v", err)
}
if !opfBreakerTripped.Load() {
t.Error("short return must trip the OPF breaker so the rewrite's post-loop check aborts the push")
Mredact/opf_test.go+2/-2
7 unmodified lines
8
9
10
11
12
13
14
15
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
19 unmodified lines
46
47
48
41
49
50
51
52
53
7 unmodified lines
// entropy or the surrounding key name, so it catches low-entropy
// credential formats the other secret layers don't reliably flag.
//
// The betterleaks layer misses these in isolation: its Supabase
// secret-key rule is a *composite* rule (RequiredRules:
// supabase-project-url) that only fires when a matching "*.supabase.co"
// URL is present in the same content, plus an entropy filter. A secret
// captured on its own therefore passes straight through.
// The betterleaks layer's coverage of these differs per prefix (verified
// against the vendored betterleaks v1.5.0 rule source):
// - sb_secret_: the supabase-project-api-key rule is a *composite* rule
// (RequiredRules: supabase-project-url) that only fires when a matching
// "*.supabase.co" URL is present in the same content, on top of an
// entropy<=4.0 filter. A secret captured on its own therefore passes
// straight through regardless of entropy.
// - sbp_: the supabase-management-token rule fires standalone (no
// RequiredRules), but only matches an exact 40-character lowercase body
// and is further filtered by entropy<=3.5 and a two-digit minimum. A
// high-entropy 40-char sbp_ token captured alone IS caught by
// betterleaks; what this layer adds for sbp_ is coverage of bodies at
// other lengths, lower entropy, or without two digits.
//
// Supabase (https://supabase.com/docs/guides/getting-started/api-keys):
// - sb_secret_... secret API key (replaces the legacy service_role
19 unmodified lines
// the {20,} length check is open-ended, sufficiently long snake_case
// identifiers that merely start with a provider prefix are redacted even
// though they aren't secrets — e.g. `sb_secret_key_rotation_handler`, or
// mid-word inside a longer identifier like `libsbp_something_long`. This is
// mid-word inside a longer identifier like
// `libsbp_something_long_enough_value`. This is
// accepted: over-redaction is the safe direction here (see
// TestString_SupabaseProviderTokenLongIdentifierOverRedaction), and adding
// anchors or capping the body length to eliminate it would reopen the
Mredact/providers.go+15/-6
345 unmodified lines
346
347
348
349
350
351
352
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
44 unmodified lines
419
420
421
409
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
345 unmodified lines
// TestString_SupabaseProviderTokens covers issue #1716: Supabase sb_secret_
// API keys and sbp_ personal access tokens are low-entropy and, captured in
// isolation, are missed by the entropy layer (threshold 4.5) and by the
// betterleaks Supabase rule (a composite rule that only fires when a
// *.supabase.co URL is co-present). The deterministic provider-prefix layer
// must catch them regardless of entropy or the surrounding variable name.
// isolation, are missed by the entropy layer (threshold 4.5). betterleaks
// coverage differs per prefix: its sb_secret_ rule is a composite rule that
// only fires when a *.supabase.co URL is co-present, so a bare sb_secret_
// value never reaches its filter at all; its sbp_ rule fires standalone but
// requires an exact 40-character lowercase body, so bodies of another length
// (like the probe values below) never match its regex regardless of entropy.
// The deterministic provider-prefix layer must catch both regardless of
// entropy, body length, or the surrounding variable name.
func TestString_SupabaseProviderTokens(t *testing.T) {
t.Parallel()
secret := supabaseSecretPrefix() + "probe_20260710_7f91c2d8e4a6b3f0" // entropy 4.199
realSecret := supabaseSecretPrefix() + "9uM4GhB0STF5R4K3HxQtlg_bzWW6DRj"
sbpToken := supabasePersonalPrefix() + "test_probe_20260710_test_probe_2026071"
// Real Supabase key bodies are base64url, which includes '-'. No other
// fixture in this test contains a hyphen, so the charset's '-' member is
// otherwise unpinned: narrowing [A-Za-z0-9_-] / [a-z0-9_-] to drop the
// hyphen would still pass every other case here while silently truncating
// (not merely shrinking) the match at the first hyphen in a real key,
// leaking the remainder raw — the #1716 failure mode recurring via an
// innocent charset "tidy-up".
secretWithHyphen := supabaseSecretPrefix() + "probe-20260710-7f91c2d8e4a6b3f0"
sbpTokenWithHyphen := supabasePersonalPrefix() + "probe-20260710-7f91c2d8e4a6b3f0"
// Both probe values sit below the entropy threshold, proving entropy-only
// detection would miss them (the issue reports entropy 4.199 for sb_secret_).
44 unmodified lines
want: `SUPABASE_SERVICE_ROLE_KEY="REDACTED"`,
},
{
name: "sbp_ personal access token (low entropy, betterleaks misses)",
name: "sbp_ personal access token (38-char body, betterleaks' rule requires exactly 40)",
input: "SUPABASE_ACCESS_TOKEN=" + sbpToken,
want: "SUPABASE_ACCESS_TOKEN=REDACTED",
},
{
name: "sb_secret_ body with an early hyphen (real base64url shape)",
input: secretWithHyphen,
want: "REDACTED",
},
{
name: "sbp_ body with an early hyphen (real base64url shape)",
input: sbpTokenWithHyphen,
want: "REDACTED",
},
})
}
// TestString_SupabaseProviderTokenLengthBoundaries pins the {20,} body-length
// floor shared by both provider patterns as an explicit boundary rather than
// an emergent property of an unrelated fixture: a body of exactly 20 chars
// must redact, and a body of exactly 19 chars must be preserved. Before this
// test, the floor was pinned only accidentally — via key_rotation_handler
// (sb_secret_) happening to have a 20-char body, with no equivalent coverage
// for sbp_ at all. Each case fails if either pattern's minimum is tightened
// to {21,}.
func TestString_SupabaseProviderTokenLengthBoundaries(t *testing.T) {
t.Parallel()
const (
body20 = "boundary_probe_2026x" // exactly 20 chars
body19 = "boundary_probe_2026" // exactly 19 chars
)
if len(body20) != 20 || len(body19) != 19 {
t.Fatalf("fixture bodies are %d/%d chars, want 20/19", len(body20), len(body19))
}
secret20 := supabaseSecretPrefix() + body20
secret19 := supabaseSecretPrefix() + body19
sbp20 := supabasePersonalPrefix() + body20
sbp19 := supabasePersonalPrefix() + body19
assertStringRedactionCases(t, []stringRedactionCase{
{
name: "sb_secret_ with exactly 20-char body redacts",
input: secret20,
want: "REDACTED",
},
{
name: "sb_secret_ with exactly 19-char body is preserved",
input: secret19,
want: secret19,
},
{
name: "sbp_ with exactly 20-char body redacts",
input: sbp20,
want: "REDACTED",
},
{
name: "sbp_ with exactly 19-char body is preserved",
input: sbp19,
want: sbp19,
},
})
}
Mredact/redact_test.go+76/-5