Skip to content

feat(redact): add OpenAI Privacy Filter as optional 8th detection layer#1214

Open
peyton-alt wants to merge 11 commits into
mainfrom
feat/openai-privacy-filter
Open

feat(redact): add OpenAI Privacy Filter as optional 8th detection layer#1214
peyton-alt wants to merge 11 commits into
mainfrom
feat/openai-privacy-filter

Conversation

@peyton-alt
Copy link
Copy Markdown
Contributor

@peyton-alt peyton-alt commented May 14, 2026

Summary

Opt-in 8th redaction layer that shells out to the user-installed OpenAI Privacy Filter (opf) binary. Runs only at condensation + export boundaries; per-turn writes stay on the existing 7-layer pipeline. Default-off; users opt in via redaction.openai_privacy_filter.enabled = true.

This PR was rewritten from main on 2026-05-14. The first iteration accumulated review-fix scope across multiple bot review passes and reached ~3000 insertions. The rewrite keeps every real correctness/privacy fix found during that cycle while skipping speculative scope (~1547 insertions, 4 commits, 19 files). The original work is preserved at feat/openai-privacy-filter-v1-backup for reference.

What's included

  • Two public entry points (StringWithPrivacyFilter, JSONLContentWithPrivacyFilter) plus thin Bytes wrappers — four plain entry points unchanged so per-turn temp writes never invoke OPF
  • \x1e-joined batched inference: one shell-out per scope amortizes the OPF cold-start across the whole transcript
  • Process-scoped circuit breaker: first OPF failure disables it for the rest of the invocation (closes OPF: process-scoped circuit breaker after first failure #1218)
  • Split createRedactedBlobFromFile so per-turn temp writes (addDirectoryToChanges) stay on the plain pipeline while copyMetadataDir uses the OPF-enabled variant
  • PromptsRedactedContent field on Write/UpdateCommittedOptions: pre-compute the joined-prompt redaction once in finalizeAllTurnCheckpoints and condenseSingleCheckpoint so multi-checkpoint commits don't re-run OPF N×
  • Settings layer rejects unknown category names at parse time — silent zero-detection of a privacy category is effectively a correctness bug
  • Sanitized stdout/stderr in shellout errors (OPF could echo input content; bytes-only diagnostics)
  • charToByteOffset correct for multibyte UTF-8 (no off-by-one at end)
  • Span docs reflect byte offsets (shellout adapter converts from OPF's character offsets)
  • Inline shell-out runtime, label map, and progress UX in redact/opf.go — no redact/opf_runtime/ subpackage, no separate progress writer

What's intentionally NOT included (deferred to follow-ups)

  • Summary-field batching via a StringsWithPrivacyFilter API — summaries are opt-in (only when IsSummarizeEnabled), have 5–10 short fields, and add ~10s worst case. Acceptable until someone complains.
  • doctor_bundle per-entry batching — diagnostic command, runs rarely, worst case slow not broken.
  • on_failure settings field — dropped entirely. DisallowUnknownFields rejects any user who tries to set it. Warn is the only supported mode today; if block-mode runtime wiring lands later, the field comes back in lockstep.

Test plan

  • mise run check (fmt + lint + test:ci + Vogon canary + roger-roger external-agent canary) — all green on 672044c2f
  • Real-agent smoke test on a fresh repo with private_person enabled — confirmed:
    • No OpenAI Privacy Filter: scanning transcript during the agent turn (per-turn writes don't invoke OPF)
    • → scanning transcript… ✓ done (29.4s) then (2.3s) at commit time (transcript + joined-prompts passes)
    • [REDACTED_PERSON] appears in entire checkpoint explain HEAD for both the prompt and the assistant transcript

🤖 Generated with Claude Code

@peyton-alt peyton-alt requested a review from a team as a code owner May 14, 2026 18:00
Copilot AI review requested due to automatic review settings May 14, 2026 18:00
Comment thread cmd/entire/cli/checkpoint/committed.go Outdated
Comment thread redact/opf_progress.go Outdated
Comment thread redact/opf_runtime/shellout.go Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (2)

redact/opf_runtime/shellout.go:199

  • charToByteOffset accepts an offset that is one rune past the end: for a 3-rune string, charOff == 4 reaches byteOff == len(s) and returns len(s) instead of -1. That lets malformed OPF spans at EOF be treated as valid and can over-redact the last input in a batch; only charOff == runeCount should map to len(s).
	for i := range charOff {
		if byteOff >= len(s) {
			if i == charOff-1 && byteOff == len(s) {
				return byteOff
			}

cmd/entire/cli/checkpoint/committed.go:1778

  • createRedactedBlobFromFile is shared by the temporary shadow-branch metadata writers (temporary.go:987 and temporary.go:1046). Switching this shared helper to the OPF-enabled redactors makes per-turn temporary metadata writes invoke OPF, contrary to the new design that keeps OPF at condensation/export boundaries and potentially adding OPF latency to every turn. Split the helper or pass a redaction mode so temporary paths continue using the plain redactors.
		redacted, jsonlErr := redact.JSONLBytesWithPrivacyFilter(ctx, content)
		if jsonlErr != nil {
			content = redact.BytesWithPrivacyFilter(ctx, content)

Comment thread redact/opf_runtime/shellout.go Outdated
Comment thread redact/opf_runtime/runtime.go Outdated
Comment thread cmd/entire/cli/settings/settings.go Outdated
Comment thread redact/opf.go Outdated
Comment thread cmd/entire/cli/checkpoint/committed.go Outdated
Comment thread cmd/entire/cli/strategy/manual_commit_hooks.go Outdated
peyton-alt added a commit that referenced this pull request May 14, 2026
Cursor Bugbot HIGH: split createRedactedBlobFromFile so per-turn temporary
writes use the plain 7-layer pipeline while committed writes use the full
8-layer pipeline (including OPF). The shared helper had silently leaked
OPF into per-turn writes via addDirectoryToChanges/addDirectoryToEntries.

Copilot Critical:
- on_failure enum validation now runs on every settings load path
  (LoadFromBytes + loadFromFile), not only the merge path.
- Parse-error path in shellOut no longer embeds stdout.String() in the
  returned error so transcript fragments don't leak to logs or TTY.

Copilot Important:
- Hoist joined-prompt redaction out of finalizeAllTurnCheckpoints' per-
  checkpoint loop and per-prompt loop; pre-compute once and pass through
  via PromptsRedactedContent. Drops OPF calls on prompts from
  len(prompts) + 2N (N=checkpoint count) to 1 per finalize.
- Same one-shot pre-redaction applied to condenseSingleCheckpoint so v1
  and v2 writers reuse a single OPF result per checkpoint.
- Span doc corrected: boundaries are byte offsets (shellout adapter
  translates from OPF's character offsets before returning Spans).

Cursor Bugbot Low: charToByteOffset no longer returns len(s) for
charOff == runeCount+1; tests pin the end-of-string and past-end cases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: eed66017e863
@peyton-alt peyton-alt requested a review from Copilot May 14, 2026 20:17
@peyton-alt
Copy link
Copy Markdown
Contributor Author

@BugBot review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1012600. Configure here.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 10 comments.

Comment thread redact/opf_runtime/shellout.go Outdated
Comment thread cmd/entire/cli/settings/settings.go Outdated
Comment thread redact/opf.go Outdated
Comment thread cmd/entire/cli/checkpoint/committed.go Outdated
Comment thread cmd/entire/cli/doctor_bundle.go Outdated
Comment thread cmd/entire/cli/settings/settings.go Outdated
Comment thread docs/security-and-privacy.md Outdated
Comment thread redact/opf.go Outdated
Comment thread cmd/entire/cli/checkpoint/temporary.go Outdated
Comment thread redact/opf.go Outdated
peyton-alt added a commit that referenced this pull request May 14, 2026
Privacy / safety:
- shellout: sanitize the cmd-run exit-error path (line 146) — stderr can
  contain echoed input from a misconfigured opf wrapper, so embedding it
  verbatim would leak transcript content via .entire/logs and the TTY.
  Report exit failure + stderr byte count instead. Mirrors the existing
  parse-error treatment.

Validation / UX:
- settings: reject unknown opf categories (e.g. "private_peerson") at
  parse time via redact.IsKnownOPFCategory. Previously a typo left OPF
  "enabled" but with zero detections and no feedback. Closed against the
  canonical label map in redact/opf.go.

Reliability:
- redact: process-scoped circuit breaker (opfBreakerTripped atomic.Bool).
  First detectOPF failure trips it; subsequent calls short-circuit before
  shelling out. One broken OPF install used to mean N × 30s timeouts
  per commit/bundle — now it's one warning plus graceful fallback.
  Reconfigure / ResetOPFConfigForTest clear the breaker so a fresh
  process retries.

Perf:
- redact: new public StringsWithPrivacyFilter([]string) []string that
  batches N inputs into a single RedactBatch call. Mirrors the
  JSONLContentWithPrivacyFilter design (has-space filter + dedupe + one
  inference pass + per-input span distribution).
- checkpoint.redactSummary: flatten Intent/Outcome/Friction/OpenItems/
  Learnings.{Repo,Workflow,Code.Finding} into one batched call. A summary
  with several Friction or Code entries used to pay the cold-start once
  per field; now once total. Preserves nil-vs-empty slice shape.

Doc / comment hygiene:
- security-and-privacy.md cost note updated to "~25–30s on CPU" (was
  "a few seconds"), now matches realistic commit behavior; mentions the
  circuit breaker.
- redact/opf.go detectOPF perf comment updated — no longer references
  the per-leaf model; references RedactBatch instead.
- handleOPFFailure TODO updated — block-mode is now rejected at settings
  parse time; relaxing that and wiring block-mode propagation must happen
  in lockstep.
- temporary.go broken doc-link replaced with a pointer to
  security-and-privacy.md (the design spec was never committed).

Tests:
- TestShellOut_ExitError_DoesNotLeakStderr pins the new stderr policy.
- TestShellOut_NonZeroExit asserts the sanitized contract (no passthrough).
- TestLoadFromBytes_RejectsUnknownCategory table-tests typo rejection.
- TestDetectOPF_CircuitBreakerSkipsAfterFirstFailure pins breaker.
- TestStringsWithPrivacyFilter_{BatchesSingleOPFCall,FallsBackOnBatchError}
  pin the batched-strings contract.
- TestRedactSummary_PreservesNilVsEmptySliceShape replaces the removed
  per-helper tests with a behavior-level assertion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: cbbcb124ec2e
peyton-alt and others added 4 commits May 14, 2026 18:23
Adds OPF as an opt-in 8th region producer in the redaction pipeline.
Two public entry points (StringWithPrivacyFilter,
JSONLContentWithPrivacyFilter) gate the OPF call; the four plain entry
points (String, Bytes, JSONLBytes, JSONLContent) are unchanged so
per-turn temp writes never invoke OPF.

Single inference pass per scope via \x1e-joined batching — opf otherwise
runs a fresh inference pass per newline-delimited input, defeating the
batch. Process-scoped atomic circuit breaker disables OPF after the
first runtime failure so a broken opf install costs one warning instead
of N×30s timeouts.

Settings layer (redaction.openai_privacy_filter) accepts enabled +
categories + command + timeout_seconds. The on_failure field is
intentionally absent: warn-only is the only mode the runtime supports
today, and DisallowUnknownFields rejects users who try to opt into a
fail-closed mode that doesn't exist. Category names are validated
against the canonical map at parse time — silent zero-detection of a
privacy category is effectively a correctness bug.

Shell-out runtime, progress UX, and label mapping are all inlined in
redact/opf.go (no separate subpackage, no separate progress writer).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Splits createRedactedBlobFromFile so per-turn temp writes (shared via
addDirectoryToChanges) stay on the plain 7-layer pipeline, and committed
writes use the full 8-layer pipeline that includes OPF. Without the
split, OPF would leak into the agent loop and add the OPF cold-start
to every per-turn write.

Adds PromptsRedactedContent to Write/UpdateCommittedOptions so the
finalize hook + single-checkpoint condense pre-compute the joined-prompt
redaction once and pass it through. Without this, each checkpoint
within a turn re-runs StringWithPrivacyFilter over identical input
(N×OPF on a turn with N checkpoints), and the v1+v2 dual-write doubles
that to 2N.

The transcript redaction in finalizeAllTurnCheckpoints and
condenseSingleCheckpoint moves to JSONLBytesWithPrivacyFilter; the
existing redactSessionJSONLBytes test seam gains a context argument so
tests can still swap a deterministic stub.

Wires settings.OpenAIPrivacyFilter into redact.ConfigurePrivacyFilter
from EnsureRedactionConfigured at startup.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an "Optional OpenAI Privacy Filter (opf)" section to
docs/security-and-privacy.md parallel to the existing "Optional PII
redaction" section: prerequisites (pip install opf), enable example,
per-category replacement-token table, full settings reference, failure
behavior (warn + circuit breaker), realistic cost (~25-30s on CPU), and
a "Verifying it's working" recipe.

Also updates the layer-count summary in the intro to mention the new
opt-in eighth pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Leftover from when assert.Contains replaced an earlier strings.Contains
call. The test build failed in test:ci with "strings" imported and not
used; this change is just removing the dead import.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@peyton-alt peyton-alt force-pushed the feat/openai-privacy-filter branch from ea9b4f5 to 672044c Compare May 14, 2026 22:52
peyton-alt and others added 7 commits May 15, 2026 11:35
Move GetOPFConfigForTest and ResetOPFConfigForTest from opf.go to
redact/export_test.go so the redact package's public API no longer
exposes mutators for the global OPF config.

Introduce redact.RedactedJoinedPrompts as a typed wrapper around the
pre-redacted joined-prompts blob written to checkpoint prompt.txt.
Construct only via redact.JoinedPrompts (runs the full 8-layer pipeline
on the joined input) or AlreadyRedactedJoinedPrompts (trusted-source
escape hatch). Rename WriteCommittedOptions.PromptsRedactedContent
(string) and the matching field on UpdateCommittedOptions to
PromptsRedacted (RedactedJoinedPrompts) so the "this content was
produced by the redaction pipeline" claim becomes a compile-time
invariant: callers cannot assign an arbitrary string. The raw
Prompts []string field gets a docstring warning that it must be
consumed only via redactJoinedPrompts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three tests in manifest_test.go hardcoded `started := time.Date(2026, 5, 8, ...)`
as the session.State.StartedAt anchor. session.StateStore.Load auto-deletes
sessions whose StartedAt is older than StaleSessionThreshold (7 days) and
returns nil, so the hardcoded date silently rots: tests pass while the
calendar is inside the 7-day window, then fail forever once it crosses.

CI on PR #964 caught this — same SHA passed yesterday (6 days after the
hardcoded date) and failed today (7 days after). Unrelated to the
streaming/diagnostic work in this PR; the manifest_test.go file isn't
touched by any other commit on this branch.

Switch all three tests to `time.Now().UTC().Add(-time.Hour)` so the
session is always one hour old at test time. Still exercises the
5-second jitter check inside matchReviewSessionState; stays well inside
the staleness window.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the OpenAI Privacy Filter from per-commit condensation to a
pre-push rewrite step. Local commits stay on the fast 7-layer pipeline;
OPF re-redacts unpushed entire/checkpoints/v1 commits right before they
leave the machine via git push.

Why: OPF inference takes ~30s per scope on CPU. Running it on every
commit interrupts the agent loop unnecessarily — most commits are
mid-session iteration. Pre-push is a much rarer event and the natural
boundary where the user has consciously decided to share content.

Architecture:
- Post-commit writes 7-layer blobs to entire/checkpoints/v1 (no OPF).
- Pre-push (when redaction.openai_privacy_filter.enabled = true):
  1. Read each unpushed v1 commit lacking Entire-OPF-Applied: true
  2. Run OPF over its blobs (full.jsonl, prompt.txt, agent-*.jsonl)
  3. Recompute content_hash.txt against the new transcript
  4. Build new commits carrying the OPF-Applied trailer
  5. Verify OPF didn't silently fall back (circuit breaker check)
  6. CAS-update the local v1 ref atomically
  7. Push the new (8-layer) commits
- Already-applied commits are re-parented without re-running OPF
  (idempotent — trailer never duplicates).
- Sentinel errors (V1DivergedError, BootstrapTooLargeError,
  V1RefMovedError, OPFRuntimeFailedError) abort the entire git push
  via non-zero hook exit. Transient checkpoint-push failures are
  logged at the CLI level and never reach the hook script, so the
  user's actual git push isn't blocked by infrastructure hiccups.

Fail-closed contract: if the OPF circuit breaker trips mid-rewrite
(runtime missing, crashed, timed out), the redact package silently
falls back to 7-layer. Without an explicit check, the rewrite would
still tag the new commits with Entire-OPF-Applied: true — future
pushes would skip them as "already done" and ship 7-layer content to
the remote forever. The rewrite now checks redact.OPFBreakerTripped()
after the rewrite loop and returns OPFRuntimeFailedError before CAS;
orphan rewritten commits sit in the object DB until git gc.

Key changes:
- redact: JoinedPromptsLegacy (no-ctx, no-OPF) for the writer safety
  net; OPFEnabled(), OPFBreakerTripped(), OPFCommand() exported
  accessors; test helpers moved out of export_test.go so cross-package
  tests can configure OPF state.
- checkpoint: redactBytesForBlob and createRedactedBlobFromFile
  collapsed to 7-layer-only (their OPF branches were dead after the
  refactor); standard WriteCommitted no longer emits the OPF-Applied
  trailer (regression-tested).
- strategy: new manual_commit_opf_rewrite.go owns the pre-push
  rewrite logic with full sentinel-error coverage. PrePush invokes it
  before pushBranchIfNeeded when OPF is enabled, logs transient
  checkpoint-push errors instead of propagating them, and returns OPF
  errors verbatim so they reach the hook exit code.
- trailers: new OPFAppliedTrailerKey constant, HasOPFApplied reader,
  and AppendOPFAppliedTrailer (idempotent) writer.
- hooks_git_cmd: pre-push command returns the strategy's error so git
  push aborts on privacy-critical failures.
- hooks.go: pre-push hook script no longer swallows non-zero exits
  (the || true was removed for pre-push specifically; other hooks
  retain their best-effort semantics).

Tests added:
- redact: JoinedPromptsLegacy parity vs pre-PR behavior; OPF never
  invoked from the legacy path.
- trailers: HasOPFApplied / AppendOPFAppliedTrailer round-trip and
  idempotency matrix.
- checkpoint: standard WriteCommitted does NOT emit OPF-Applied
  (regression guard against accidental skip of the rewrite).
- strategy: rewrite happy path (tags applied, redacts sentinel
  content), idempotent second run (no trailer duplication, tree
  preserved), missing v1 branch (no-op), diverged-remote returns
  V1DivergedError, bootstrap cap over limit returns
  BootstrapTooLargeError, bootstrap unlimited override allows
  arbitrary size, OPF runtime failure trips breaker + returns
  OPFRuntimeFailedError + leaves local ref unchanged (the fail-closed
  regression guard caught end-to-end during manual testing).

Docs:
- docs/security-and-privacy.md: pre-push flow explanation,
  persistence window for unreachable git objects (~2wk), raw
  .entire/ session-file caveat, manual scrub command, force-push
  divergence guidance, bootstrap env var.
- CLAUDE.md: strategy section notes the pre-push OPF rewrite and
  the new manual_commit_opf_rewrite.go file.

Out of scope (follow-ups):
- Interactive prompt + ENTIRE_OPF env-var gating (next commit).
- Shadow-branch cleanup post-push (commit after).
- entire doctor --recover-v1 helper referenced by the divergence
  error hint.

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

Add a 3-option prompt (Yes/No/Always) plus env-var override so users
can decide per-push whether to pay the ~30s OPF cost. Ctrl-C aborts the
push entirely.

Decision precedence (highest first):
1. ENTIRE_OPF=yes|no   — per-push override on the git push invocation
2. redaction.openai_privacy_filter.prompt_default = always|never
3. Interactive prompt (TTY + ask mode, the default)
4. Non-interactive fallback (no TTY) — auto-run with stderr progress

The "Always" choice persists prompt_default=always to
.entire/settings.local.json so future pushes don't ask. The settings
file is updated via a JSON-map round-trip so unrelated fields survive.

Architecture:
- resolveOPFDecision is pure (env, setting, hasTTY, prompter) → decision.
  Tests inject a fake prompter; production path uses the huh form.
- resolveOPFDecisionForPrePush is the production wiring: reads env,
  reads settings, checks TTY via interactive.CanPromptInteractively(),
  invokes askOPFPrompt only when needed, emits stderr progress on the
  non-TTY auto-run path.
- PrePush integrates the decision: Run → invoke rewrite; Skip → push
  7-layer as-is; Abort → return errOPFAbortedByUser (hook propagates
  non-zero, git push aborts).

Settings:
- OPFSettings.PromptDefault: "" | "ask" | "always" | "never"
- Constants exported: OPFPromptAsk, OPFPromptAlways, OPFPromptNever
- Validator rejects unknown values at parse time (matches existing
  on_failure precedent — typos surface immediately).

Tests:
- 15-case table-driven precedence (env > setting > prompt > non-TTY).
- Persistence round-trip preserves unrelated settings.
- Fresh-install path creates settings.local.json.
- Settings validator accepts ask/always/never/empty, rejects bogus.

Docs:
- security-and-privacy.md describes the prompt UX, env-var overrides,
  and non-TTY behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The tree walker was redacting every .jsonl/.txt blob in the cumulative
v1 tree of each rewritten commit, including files belonging to shards
that were pushed in prior commits (and may or may not have been
OPF-applied already). Empirically: 1 new commit with 3 pre-existing
shards in the tree → 12 OPF calls instead of 2.

With real OPF at ~30s cold start per shell-out, a typical push with
10 prior shards would burn ~5 minutes of inference time re-redacting
content that was already on the remote.

Fix: parse the checkpoint ID from the commit's "Checkpoint: <id>"
subject and only redact files at paths under the target shard
(<id[:2]>/<id[2:]>/*). Other shards in the tree are copied verbatim
into the new tree — their bytes are unchanged, so they reuse existing
blobs (and the corresponding tree nodes) without any OPF cost.

The conservative fallback (walk-everything) is preserved for commits
without a recognizable Checkpoint: subject — e.g., the
"Initialize sessions branch" bootstrap commit has an empty tree
anyway, so the fallback costs zero OPF calls.

End-to-end measurement (fake opf logs each call):
- Before: 12 calls for 1 new commit on a tree with 3 prior shards
- After: 2 calls (the commit's own full.jsonl + prompt.txt)

Tests added:
- TestParseShardPathFromCommitMessage covers the subject parser
  (valid hex id, missing prefix, wrong length, non-hex, uppercase
  rejected, empty message).
- TestShouldDescendAndInsideShard pins the recursion predicates:
  descend into ancestors/target/descendants, copy siblings; redact
  files inside the shard, copy files outside.
- TestShardScopeEmptyShardPathIsPermissive locks down the
  walk-everything fallback for bootstrap commits.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After a successful push of entire/checkpoints/v1, delete shadow
branches (entire/<commit>-<worktree>) whose sessions have all ended
cleanly. Previously these accumulated until the user ran `entire clean`
manually.

Safety properties (conservative — risk-asymmetric):
- A shadow branch is deleted ONLY when every session state file
  referencing it has EndedAt != nil AND TurnCheckpointIDs is empty.
- Active session (EndedAt == nil) protects its shadow branch.
- Mid-finalize race (ended but TurnCheckpointIDs pending) preserves
  the branch — finalize may still need it.
- Multiple sessions sharing the same shadow branch (same base commit
  + worktree) must ALL satisfy the criteria.
- Orphaned shadow branches (no associated session state file) are
  deleted — no session to lose data from.
- Cleanup only runs after pushBranchIfNeeded returns nil (v1 push
  succeeded). If push failed, shadow branches are left alone so the
  user's next push attempt can still leverage their data.
- DeleteShadowBranches failures (e.g., stale lock) are logged and
  don't abort the push — branches just remain for the next push.

Files:
- cleanup.go: new CleanupPushedShadowBranches(ctx) (~50 LOC reusing
  ListShadowBranches + ListSessionStates + DeleteShadowBranches).
- manual_commit_push.go: invoke from PrePush after successful
  pushBranchIfNeeded.
- cleanup_pushed_shadow_test.go: 6 cases covering the full predicate
  matrix (fully-ended, active-session, pending-checkpoints, orphaned,
  mixed-branches, no-branches).
- docs/security-and-privacy.md: persistence table updated to reflect
  the new auto-cleanup.

End-to-end verified: a manually-created shadow branch with a matching
ended session state file gets deleted after the next successful push;
entire/checkpoints/v1 stays (correctly excluded by IsShadowBranch).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

OPF: process-scoped circuit breaker after first failure

2 participants