refactor(redact): drop unused OPF helpers, table-drive cleanup tests

Two trim-and-simplify changes from a self-review of the PR's surface:

1. Drop redact.JoinedPromptsLegacy + its dedicated test file.
   The function was added to keep the writer's safety net 7-layer-only
   while signaling the distinction via the type system. With pre-push
   rewrite owning all OPF execution, the writer is now unambiguously
   7-layer-only — the typed-wrapper signal is no longer earning its
   weight. checkpoint.redactedJoinedPrompts now inlines
   redact.String(strings.Join(prompts, PromptSeparator)) for the same
   behavior with less ceremony.

2. Drop redact.GetOPFConfigForTest. The "ForTest" helper was exported
   for cross-package tests but never had any callers — production code
   should never introspect the OPF config directly (use OPFEnabled()),
   and our tests already use ConfigurePrivacyFilterWithRuntime +
   ResetOPFConfigForTest to manage state.

3. Table-drive the four single-shadow-branch cleanup tests into one
   TestCleanupPushedShadowBranches_Predicate. The four cases shared
   ~90% of setup; the table-driven form keeps each scenario as a named
   sub-test while collapsing the boilerplate. The two non-single-shadow
   cases (mixed branches, no branches at all) stay standalone because
   their setups are structurally different.

Net diff: -7 tests (collapsed into 4 sub-tests in 1 function), -100
lines of helpers, no behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: peyton-alt

cmd/entire/cli/checkpoint/prompts.go

@@ -31,21 +31,12 @@ func SplitPromptContent(content string) []string {
 
 // redactedJoinedPrompts returns the redacted prompt-blob content for the
 // supplied prompts. When preRedacted is set it is unwrapped verbatim;
-// otherwise the prompts are joined and run through the 7-layer pipeline
-// as a safety net.
-//
-// The safety net is intentionally 7-layer-only (no OPF), even when OPF
-// is enabled globally. OPF runs exclusively in the pre-push rewrite
-// path so per-commit condensation stays fast and predictable; the
-// safety net never drags OPF into a hot path. Callers that have
-// already produced an 8-layer blob (e.g. the pre-push rewrite itself)
-// pass it as preRedacted so this function returns it verbatim.
-//
-// ctx is retained on the signature for future extensions; the 7-layer
-// pipeline doesn't consume it today.
+// otherwise the prompts are joined and run through the 7-layer pipeline.
+// OPF runs exclusively in the pre-push rewrite (not here), so the
+// writer's hot path stays predictable.
 func redactedJoinedPrompts(_ context.Context, prompts []string, preRedacted redact.RedactedJoinedPrompts) string {
 	if preRedacted.IsSet() {
 		return preRedacted.String()
 	}
-	return redact.JoinedPromptsLegacy(prompts, PromptSeparator).String()
+	return redact.String(strings.Join(prompts, PromptSeparator))
 }

cmd/entire/cli/checkpoint/prompts_test.go

@@ -47,15 +47,14 @@ func TestRedactedJoinedPrompts_PreRedactedIsTrustedVerbatim(t *testing.T) {
 	assert.Equal(t, preRedacted, got, "preRedacted should pass through verbatim")
 }
 
-// TestRedactedJoinedPrompts_ZeroValueFallsBackToLegacyRedaction verifies
-// that when the typed preRedacted is the zero value the helper joins the
-// prompts and runs the 7-layer pipeline as a safety net. Critically, the
-// fallback is JoinedPromptsLegacy (no OPF) — OPF runs exclusively in the
-// pre-push rewrite path, never here in the writer's hot path.
-func TestRedactedJoinedPrompts_ZeroValueFallsBackToLegacyRedaction(t *testing.T) {
+// TestRedactedJoinedPrompts_ZeroValueFallsBackToSafetyNet verifies
+// that when the typed preRedacted is the zero value the helper joins
+// the prompts and runs the 7-layer pipeline as a safety net. OPF runs
+// exclusively in the pre-push rewrite path, never here.
+func TestRedactedJoinedPrompts_ZeroValueFallsBackToSafetyNet(t *testing.T) {
 	t.Parallel()
 
 	got := redactedJoinedPrompts(context.Background(), []string{"hello", "world"}, redact.RedactedJoinedPrompts{})
-	assert.NotEmpty(t, got, "zero-value preRedacted should fall back to running the legacy 7-layer pipeline")
+	assert.NotEmpty(t, got, "zero-value preRedacted should fall back to running the 7-layer pipeline")
 	assert.Contains(t, got, PromptSeparator, "fallback output should preserve the prompt separator")
 }

cmd/entire/cli/strategy/cleanup_pushed_shadow_test.go

@@ -71,57 +71,45 @@ func (e *shadowCleanupEnv) branchExists(name string) bool {
 	return err == nil
 }
 
-// Happy case: ended session with no pending checkpoints → shadow gone.
-func TestCleanupPushedShadowBranches_FullyEndedDeleted(t *testing.T) {
-	env := newShadowCleanupEnv(t)
-	shadow := env.addShadowBranch(env.baseHash.String(), "")
-	ended := time.Now().Add(-time.Minute)
-	env.addSessionState("s1", env.baseHash.String(), "", &ended, nil)
-
-	deleted, err := CleanupPushedShadowBranches(context.Background())
-	require.NoError(t, err)
-	require.Equal(t, 1, deleted)
-	require.False(t, env.branchExists(shadow))
-}
-
-// Active session (EndedAt == nil) protects the shadow even if another
-// session on the same branch already ended.
-func TestCleanupPushedShadowBranches_ActiveSessionPreserved(t *testing.T) {
-	env := newShadowCleanupEnv(t)
-	shadow := env.addShadowBranch(env.baseHash.String(), "")
+// Predicate matrix: each shadow branch is paired with zero or more
+// session states; the cleanup must respect the safety rules (active
+// session OR pending turn checkpoints protect the branch; ended-clean
+// or orphaned branches are deleted).
+func TestCleanupPushedShadowBranches_Predicate(t *testing.T) {
 	ended := time.Now().Add(-time.Minute)
-	env.addSessionState("s1-ended", env.baseHash.String(), "", &ended, nil)
-	env.addSessionState("s2-active", env.baseHash.String(), "", nil, nil)
-
-	deleted, err := CleanupPushedShadowBranches(context.Background())
-	require.NoError(t, err)
-	require.Equal(t, 0, deleted)
-	require.True(t, env.branchExists(shadow))
-}
-
-// Mid-finalize race window: ended session with TurnCheckpointIDs still
-// pending → preserve.
-func TestCleanupPushedShadowBranches_PendingTurnCheckpointsPreserved(t *testing.T) {
-	env := newShadowCleanupEnv(t)
-	shadow := env.addShadowBranch(env.baseHash.String(), "")
-	ended := time.Now().Add(-time.Minute)
-	env.addSessionState("s1", env.baseHash.String(), "", &ended, []string{"a1b2c3d4e5f6"})
-
-	deleted, err := CleanupPushedShadowBranches(context.Background())
-	require.NoError(t, err)
-	require.Equal(t, 0, deleted)
-	require.True(t, env.branchExists(shadow))
-}
-
-// Orphaned shadow branch (no matching session state) gets cleaned up.
-func TestCleanupPushedShadowBranches_OrphanedBranchDeleted(t *testing.T) {
-	env := newShadowCleanupEnv(t)
-	shadow := env.addShadowBranch(env.baseHash.String(), "")
-
-	deleted, err := CleanupPushedShadowBranches(context.Background())
-	require.NoError(t, err)
-	require.Equal(t, 1, deleted)
-	require.False(t, env.branchExists(shadow))
+	type sessionFixture struct {
+		id                string
+		ended             *time.Time
+		pendingCheckpoint []string
+	}
+	cases := []struct {
+		name        string
+		sessions    []sessionFixture
+		wantDeleted bool
+	}{
+		{name: "ended_no_pending_deleted", sessions: []sessionFixture{{id: "s1", ended: &ended}}, wantDeleted: true},
+		{name: "active_session_preserved", sessions: []sessionFixture{{id: "s1", ended: &ended}, {id: "s2", ended: nil}}, wantDeleted: false},
+		{name: "pending_turn_checkpoints_preserved", sessions: []sessionFixture{{id: "s1", ended: &ended, pendingCheckpoint: []string{"a1b2c3d4e5f6"}}}, wantDeleted: false},
+		{name: "orphaned_branch_no_sessions_deleted", sessions: nil, wantDeleted: true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			env := newShadowCleanupEnv(t)
+			shadow := env.addShadowBranch(env.baseHash.String(), "")
+			for _, s := range tc.sessions {
+				env.addSessionState(s.id, env.baseHash.String(), "", s.ended, s.pendingCheckpoint)
+			}
+			deleted, err := CleanupPushedShadowBranches(context.Background())
+			require.NoError(t, err)
+			if tc.wantDeleted {
+				require.Equal(t, 1, deleted)
+				require.False(t, env.branchExists(shadow))
+			} else {
+				require.Equal(t, 0, deleted)
+				require.True(t, env.branchExists(shadow))
+			}
+		})
+	}
 }
 
 // Mixed: two shadow branches with different worktree IDs and different

cmd/entire/cli/strategy/manual_commit_hooks.go

@@ -2760,7 +2760,7 @@ func (s *ManualCommitStrategy) finalizeAllTurnCheckpoints(ctx context.Context, s
 	}
 
 	// Post-commit emits 7-layer-only blobs; the writer's safety net
-	// (checkpoint.redactedJoinedPrompts via JoinedPromptsLegacy) handles
+	// (checkpoint.redactedJoinedPrompts via redact.String) handles
 	// joining + 7-layer redaction. OPF is applied later, once per push,
 	// by the pre-push rewrite path.
 	redactedPrompts := redact.RedactedJoinedPrompts{}

redact/joined_prompts_test.go

@@ -155,23 +155,6 @@ func AlreadyRedactedJoinedPrompts(content string) RedactedJoinedPrompts {
 	return RedactedJoinedPrompts{content: content}
 }
 
-// JoinedPromptsLegacy joins prompts with sep and runs only the seven
-// always-on/opt-in regex layers — never the OpenAI Privacy Filter, even
-// when OPF is configured globally. Used by the checkpoint writer's
-// safety net so post-commit blobs stay on the fast 7-layer pipeline; OPF
-// is applied exclusively by the pre-push rewrite path (see
-// strategy/manual_commit_opf_rewrite.go), which calls JoinedPrompts.
-//
-// Mirrors JoinedPrompts in return type so callers can use either
-// constructor and feed the result to WriteCommittedOptions.PromptsRedacted
-// without changing the type contract.
-func JoinedPromptsLegacy(prompts []string, sep string) RedactedJoinedPrompts {
-	if len(prompts) == 0 {
-		return RedactedJoinedPrompts{}
-	}
-	return RedactedJoinedPrompts{content: String(strings.Join(prompts, sep))}
-}
-
 var (
 	betterleaksDetector     *detect.Detector
 	betterleaksDetectorOnce sync.Once

redact/redact.go

@@ -1,25 +1,11 @@
 package redact
 
-// This file exposes internal helpers needed by tests in OTHER packages
-// (e.g. cmd/entire/cli/strategy) that exercise OPF behavior end-to-end.
-// They cannot live in export_test.go because that file is invisible to
-// importers — only redact's own tests would be able to call them.
-//
-// The "ForTest" suffix is a convention signal: production code MUST NOT
-// call these. A future lint rule (e.g. forbidigo on the "ForTest"
-// suffix) could enforce that statically; today it relies on reviewer
-// discipline.
+// Cross-package test helpers. Lives in a regular .go file (not
+// export_test.go) so tests in cmd/entire/cli/strategy can call it.
+// The "ForTest" suffix is the production-code-must-not-call signal.
 
-// GetOPFConfigForTest returns the current OPF configuration, or nil if
-// never configured. Test-only — production code should never need to
-// introspect the global config; gate behavior on OPFEnabled() instead.
-func GetOPFConfigForTest() *OPFConfig {
-	return getOPFConfig()
-}
-
-// ResetOPFConfigForTest clears configuration and the circuit breaker.
-// Used to return to a "never configured" state between test cases.
-// Test-only — production code should never need to reset OPF state.
+// ResetOPFConfigForTest clears OPF configuration and the circuit
+// breaker. Test-only.
 func ResetOPFConfigForTest() {
 	resetOPFConfig()
 }