address remaining OPF review feedback · Entire

Address Remaining OPF Review Feedback

bf3c9f2→main·

peyton-alt·1mo ago·6 files·+263 added/-28 removed

Sessions

1d00e2e4a43dView transcript

[?
Address OPF Review Feedback and TestingCodex·GPT-5.5·4 steps](/content/gh/entireio/cli/session/019ed7c0-d406-7743-bc93-5b4c4efc5c45#timeline-1d00e2e4a43d/index.html)

Changes

6

168 unmodified lines

169
170
171
172
173
172
173
174
175
175
176
176
177
178
28 unmodified lines

207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
11 unmodified lines

240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266

168 unmodified lines

// because at least one session still depends on them.
    protected := map[string]bool{}
    for _, s := range states {
        if s.Phase == session.PhaseEnded && s.FullyCondensed && len(s.TurnCheckpointIDs) == 0 {
            continue // safe — session ended cleanly and finalized
        }
        shadow, ok := protectedShadowBranchForSession(s)
        if ok {
            protected[shadow] = true
        }
        shadow := getShadowBranchNameForCommit(s.BaseCommit, s.WorktreeID)
        protected[shadow] = true
    }

toDelete := map[string]plumbing.Hash{}
28 unmodified lines

failed = append(failed, branch)
        continue
    }
    protected, err := shadowBranchProtectedByCurrentState(ctx, branch)
    if err != nil {
        logging.Debug(ctx, "shadow branch unchanged-delete skipped after protection recheck failed",
            slog.String("branch", branch),
            slog.String("error", err.Error()),
        )
        failed = append(failed, branch)
        continue
    }
    if protected {
        logging.Debug(ctx, "shadow branch unchanged-delete skipped because current session state protects it",
            slog.String("branch", branch),
        )
        failed = append(failed, branch)
        continue
    }
    ref := "refs/heads/" + branch
    cmd := exec.CommandContext(ctx, "git", "update-ref", "-d", ref, expected.String())
    if output, runErr := cmd.CombinedOutput(); runErr != nil {
11 unmodified lines

return deleted, failed
}

func protectedShadowBranchForSession(s *SessionState) (string, bool) {
    if s.Phase == session.PhaseEnded && s.FullyCondensed && len(s.TurnCheckpointIDs) == 0 {
        return "", false // safe — session ended cleanly and finalized
    }
    return getShadowBranchNameForCommit(s.BaseCommit, s.WorktreeID), true
}

func shadowBranchProtectedByCurrentState(ctx context.Context, branch string) (bool, error) {
    states, err := ListSessionStates(ctx)
    if err != nil {
        return false, err
    }
    for _, s := range states {
        shadow, ok := protectedShadowBranchForSession(s)
        if ok && shadow == branch {
            return true, nil
        }
    }
    return false, nil
}

// DeleteShadowBranches deletes the specified branches from the repository.
// Returns two slices: successfully deleted branches and branches that failed to delete.
// Individual branch deletion failures do not stop the operation - all branches are attempted.

Mcmd/entire/cli/strategy/cleanup.go+40/-4

169 unmodified lines

170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187

169 unmodified lines

require.NoError(t, err)
    require.Equal(t, newHash, ref.Hash())
}

func TestDeleteShadowBranchesIfUnchanged_PreservesBranchProtectedAfterSnapshot(t *testing.T) {
    env := newShadowCleanupEnv(t)
    shadow := env.addShadowBranch(env.baseHash.String(), "")
    snapshot := map[string]plumbing.Hash{
        shadow: env.baseHash,
    }

env.addSessionState("s-race", env.baseHash.String(), "", nil, nil, false)

deleted, failed := DeleteShadowBranchesIfUnchanged(context.Background(), snapshot)
    require.Empty(t, deleted)
    require.Equal(t, []string{shadow}, failed)
    require.True(t, env.branchExists(shadow))
}

Mcmd/entire/cli/strategy/cleanup_pushed_shadow_test.go+15

195 unmodified lines

196
197
198
199
200
201
202
203
204
205
199
200
201
202
203
204
205
206
207
208
209
210
212
213
214
211
212
213
214
215
216
217
218
219
220
221
222

195 unmodified lines

// resolveRemoteV1Tip returns the hash of the remote's
// entire/checkpoints/v1 tip.
//
// When target is a remote name (e.g., "origin"), looks up the local
// tracking ref `refs/remotes/<target>/entire/checkpoints/v1`. When
// target is a URL (checkpoint_remote configured), fetches the v1 ref
// from the URL into a temporary local ref so the rewrite can see what's
// already on the remote — otherwise every push would re-redact the
// entire history as a "bootstrap" since URL-based remotes have no
// tracking refs locally.
// Fetches the v1 ref from target into a temporary local ref so the
// rewrite compares against the current remote tip rather than a stale
// remote-tracking ref. target may be either a remote name (e.g. "origin")
// or a URL (checkpoint_remote configured). Fetching is especially important
// for URL-based remotes, which have no tracking refs locally; otherwise every
// push would re-redact the entire history as a "bootstrap."
//
// Returns ZeroHash with no error when the remote has no v1 yet (genuine
// bootstrap case). Fetch failures fall back to ZeroHash + a warning
// log; the rewrite then treats the push as bootstrap rather than
// blocking the user on a transient network issue.
func resolveRemoteV1Tip(ctx context.Context, repo *git.Repository, target string) (plumbing.Hash, error) {
    if !remote.IsURL(target) {
        return readV1Tip(repo, plumbing.NewRemoteReferenceName(target, paths.MetadataBranchName))
    }
    srcRef := "refs/heads/" + paths.MetadataBranchName
    if err := fetchURLIntoTmpRef(ctx, target, srcRef, opfRewriteFetchTmpRef, "v1 for OPF rewrite", true); err != nil {
        if !remote.IsURL(target) {
            logging.Warn(ctx, "OPF rewrite: failed to fetch remote v1; using local remote-tracking ref",
                slog.String("remote", target),
                slog.String("error", err.Error()),
            )
            return readV1Tip(repo, plumbing.NewRemoteReferenceName(target, paths.MetadataBranchName))
        }
        logging.Warn(ctx, "OPF rewrite: failed to fetch remote v1 from URL; treating push as bootstrap",
                slog.String("error", err.Error()),
            )
    }
}

Mcmd/entire/cli/strategy/manual_commit_opf_rewrite.go+13/-10

16 unmodified lines

17
18
19
20
21
22
23
290 unmodified lines

314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349

16 unmodified lines

"github.com/entireio/cli/cmd/entire/cli/trailers"
    "github.com/entireio/cli/redact"
    "github.com/go-git/go-git/v6"
    gitconfig "github.com/go-git/go-git/v6/config"
    "github.com/go-git/go-git/v6/plumbing"
    "github.com/go-git/go-git/v6/plumbing/filemode"
    "github.com/go-git/go-git/v6/plumbing/object"
290 unmodified lines

require.Equal(t, remoteTip, diverged.Remote)
}

func TestResolveRemoteV1Tip_NamedRemoteFetchesLatestTip(t *testing.T) {
    localDir := t.TempDir()
    remoteDir := t.TempDir()
    testutil.InitRepo(t, localDir)
    testutil.InitRepo(t, remoteDir)
    t.Chdir(localDir)

localRepo, err := git.PlainOpen(localDir)
    require.NoError(t, err)
    remoteRepo, err := git.PlainOpen(remoteDir)
    require.NoError(t, err)

remoteTree := emptyTreeHash(t, remoteRepo)
    staleRemoteTip := makeOrphanCommit(t, remoteRepo, remoteTree, nil, "stale remote checkpoint tip")
    latestRemoteTip := makeOrphanCommit(t, remoteRepo, remoteTree, []plumbing.Hash{staleRemoteTip}, "latest remote checkpoint tip")
    require.NoError(t, remoteRepo.Storer.SetReference(
        plumbing.NewHashReference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), latestRemoteTip)))

cfg, err := localRepo.Config()
    require.NoError(t, err)
    cfg.Remotes["origin"] = &gitconfig.RemoteConfig{Name: "origin", URLs: []string{remoteDir}}
    require.NoError(t, localRepo.SetConfig(cfg))
    require.NoError(t, localRepo.Storer.SetReference(
        plumbing.NewHashReference(plumbing.NewRemoteReferenceName("origin", paths.MetadataBranchName), staleRemoteTip)))

got, err := resolveRemoteV1Tip(context.Background(), localRepo, "origin")
    require.NoError(t, err)
    require.Equal(t, latestRemoteTip, got)
}

// Bootstrap cap: a single table-driven test covers both the over-limit
// rejection and the unlimited-override pass paths since they share
// 90% of setup.

Mcmd/entire/cli/strategy/manual_commit_opf_rewrite_test.go+31

302 unmodified lines

303
304
305
306
307
308
309
310
311
312
313
314
315
316
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
24 unmodified lines

351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
11 unmodified lines

378
379
380
369
370
371
381
382
383
384
385
386
387
2 unmodified lines

390
391
392
393
394
395
396
397
398
399
400
401
5 unmodified lines

407
408
409
410
411
412
413
414
415
416
417
418
27 unmodified lines

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
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511

302 unmodified lines

}
}

// opfBatchSeparator joins inputs into a single opf invocation. opf treats
// '\n' as a per-input delimiter and runs a fresh inference pass per line,
// which is no faster than per-call shell-out. Joining with a non-newline
// separator instead causes opf to treat the concatenation as ONE input and
// do ONE inference pass, amortizing the model load across all inputs.
//
// ASCII RECORD SEPARATOR (U+001E) satisfies both requirements:
//  1. Doesn't appear in real text (so no collision with content)
//  2. Looks like whitespace to opf's tokenizer (so it doesn't confuse
//     span boundaries)
const opfBatchSeparator = "\x1e"
const (
    // opfBatchSeparator joins inputs into a single opf invocation. opf treats
    // '\n' as a per-input delimiter and runs a fresh inference pass per line,
    // which is no faster than per-call shell-out. Joining with a non-newline
    // separator instead causes opf to treat the concatenation as ONE input and
    // do ONE inference pass, amortizing the model load across all inputs.
    //
    // ASCII RECORD SEPARATOR (U+001E) satisfies both requirements:
    //  1. Doesn't appear in real text (so no collision with content)
    //  2. Looks like whitespace to opf's tokenizer (so it doesn't confuse
    //     span boundaries)
    opfBatchSeparator = "\x1e"

// Keep a pathological transcript or process from making the CLI allocate
    // unbounded buffers while preparing or reading an OPF shell-out.
    opfMaxBatchInputBytes    = 16 * 1024 * 1024
    opfMaxProcessOutputBytes = 1 * 1024 * 1024
)

// Redact runs OPF on a single text input.
func (s *shellOut) Redact(ctx context.Context, text string, categories []string) ([]Span, error) {
24 unmodified lines

if len(inputs) == 0 || len(categories) == 0 {
        return nil, nil
    }
    batchedLen, tooLarge := opfBatchedInputLen(inputs)
    if tooLarge {
        return nil, fmt.Errorf("opf input too large (%d bytes, limit %d)", batchedLen, opfMaxBatchInputBytes)
    }
    timeout := time.Duration(s.timeoutSeconds) * time.Second
    callCtx, cancel := context.WithTimeout(ctx, timeout)
    defer cancel()

var buf strings.Builder
    buf.Grow(batchedLen)
    starts := make([]int, len(inputs))
    for i, in := range inputs {
        if i > 0 {
11 unmodified lines

"--no-print-color-coded-text",
    )
    cmd.Stdin = strings.NewReader(batched)
    var stdout, stderr bytes.Buffer
    cmd.Stdout = &stdout
    cmd.Stderr = &stderr
    stdout := newLimitedOutputBuffer(opfMaxProcessOutputBytes)
    stderr := newLimitedOutputBuffer(opfMaxProcessOutputBytes)
    cmd.Stdout = stdout
    cmd.Stderr = stderr
    // WaitDelay forces cmd.Wait() to return promptly after context
    // cancellation/timeout even when the killed process leaves descendants
    // holding the stdout/stderr pipes (e.g. `sh -c "sleep 5" — killing sh
    2 unmodified lines

cmd.WaitDelay = 500 * time.Millisecond

if err := cmd.Run(); err != nil {
        if stdout.Exceeded() {
            return nil, fmt.Errorf("opf stdout exceeded %d byte limit", stdout.Limit())
        }
        if stderr.Exceeded() {
            return nil, fmt.Errorf("opf stderr exceeded %d byte limit", stderr.Limit())
        }
        switch {
        case errors.Is(callCtx.Err(), context.DeadlineExceeded):
            return nil, fmt.Errorf("opf timeout after %s: %w", timeout, callCtx.Err())
        }
        return nil, fmt.Errorf("opf exited with error (%d bytes on stderr): %w", stderr.Len(), err)
    }
    if stdout.Exceeded() {
        return nil, fmt.Errorf("opf stdout exceeded %d byte limit", stdout.Limit())
    }
    if stderr.Exceeded() {
        return nil, fmt.Errorf("opf stderr exceeded %d byte limit", stderr.Limit())
    }

var parsed struct {
27 unmodified lines

return out, nil
}

func opfBatchedInputLen(inputs []string) (int, bool) {
    total := 0
    for i, in := range inputs {
        if i > 0 {
            if total > opfMaxBatchInputBytes-len(opfBatchSeparator) {
                return total + len(opfBatchSeparator), true
            }
            total += len(opfBatchSeparator)
        }
        if total > opfMaxBatchInputBytes-len(in) {
            return total + len(in), true
        }
        total += len(in)
    }
    return total, false
}

type limitedOutputBuffer struct {
    buf      bytes.Buffer
    limit    int
    exceeded bool
}

func newLimitedOutputBuffer(limit int) *limitedOutputBuffer {
    return &limitedOutputBuffer{limit: limit}
}

func (b *limitedOutputBuffer) Write(p []byte) (int, error) {
    if b.limit <= 0 {
        b.exceeded = true
        return len(p), nil
    }
    remaining := b.limit - b.buf.Len()
    if remaining > 0 {
        if len(p) <= remaining {
            _, _ = b.buf.Write(p)
            return len(p), nil
        }
        _, _ = b.buf.Write(p[:remaining])
        }
    b.exceeded = true
    return len(p), nil
}

func (b *limitedOutputBuffer) Bytes() []byte {
    return b.buf.Bytes()
}

func (b *limitedOutputBuffer) Len() int {
    return b.buf.Len()
}

func (b *limitedOutputBuffer) Limit() int {
    return b.limit
}

func (b *limitedOutputBuffer) Exceeded() bool {
    return b.exceeded
}

func sanitizeOPFBatchInput(in string) string {
    replacer := strings.NewReplacer("\n", " ", opfBatchSeparator, " ")
    return replacer.Replace(in)
}