Route OPF Progress to stderr

I have the full picture. Here's the explanation.

Why you see this output

The lines come from the redact package's OPF progress UX (redact/opf.go, redact/batch.go, redact/redact.go). The key detail is where they're written:

opfStderr is wired to /dev/tty, not stdout or stderr. That's deliberate for production: the post-commit hook runs with 2>/dev/null, so writing to the controlling terminal directly lets the "scanning…/done" progress survive that redirect.

The side effect: because it goes straight to your terminal, it bypasses go test's output capture and shows up interleaved with PASS lines.

The tests producing it are the strategy-package OPF tests (manual_commit_opf_rewrite_test.go and friends). They configure a fake OPF runtime via configureFakeOPF:

…but unlike the redact package's own tests (which set opfStderr = io.Discard), these tests never silence opfStderr. So the progress messages leak to /dev/tty. The × unavailable (/tmp/test-opf) line is the fail-closed test exercising the "OPF binary missing → fall back to regex layers" path; /tmp/test-opf is a deliberately bogus command path.

Is it doing anything expensive?

No. Everything you saw is cheap, in-memory work:

• The runtime is a fake stub — RedactBatch just returns precomputed spans (e.g. findSentinelSpans). No subprocess, no Python/opf binary, no model load, no network. The 0.0s timings confirm it.
• The pip install opf / unavailable line is a fallback path — it does less (skips OPF, runs only regex layers over a few tiny test blobs).
• Real OPF (what users get when redaction.openai_privacy_filter.enabled is true) shells out to a Python process and can take ~30s — that's why the real pre-push prompt warns "may take ~30s." But that real path is never hit here.

So this is harmless cosmetic leakage, not a performance concern.

If you want to silence it

The clean fix is to make configureFakeOPF redirect opfStderr to io.Discard for the duration of the test (matching what redact's own tests already do), via a small exported test helper in redact — there's currently no exported setter, only the unexported var. Want me to add that helper and route the strategy tests through it?

Yes — they differ in two independent ways, and that's the whole reason for the /dev/tty trick.

1. OPF doesn't run at post-commit at all

Post-commit redaction goes through createRedactedBlobFromFile, which is explicitly the 7-layer pipeline. The comment spells it out (committed.go:1887):

The OpenAI Privacy Filter is intentionally NOT run here — OPF lives in the pre-push rewrite path… which re-redacts the 7-layer blobs into 8-layer commits before they leave the local machine.

So in production the OPF "scanning…/done" progress is inherently a pre-push-only thing. Per-commit condensation never emits it because OPF (the slow, potentially-~30s subprocess) never runs there.

2. The two hooks are installed with different shell wrappers

From buildHookSpecs (hooks.go:177-199):

• post-commit: 2>/dev/null hides all stderr logging, and || true means a failure can never block the commit. Quiet and non-fatal by design.
• pre-push: deliberately no 2>/dev/null and no || true. Its stderr reaches the terminal, and a non-zero exit aborts git push — the fail-closed contract for OPF (diverged remote, oversized bootstrap, CAS conflict, runtime failure all must stop the push).

How they connect — why OPF writes to /dev/tty

This is the key insight tying it to your earlier question. opfStderr targets /dev/tty rather than stderr precisely so the progress UX survives a stderr-swallowing hook (opf.go:185):

Defaults to /dev/tty so messages survive the post-commit hook's stderr redirect (the hook runs entire hooks git post-commit 2>/dev/null…).

So the design is belt-and-suspenders: OPF only runs at pre-push (where stderr isn't swallowed anyway), but its messages still bypass any 2>/dev/null by going straight to the controlling terminal. That same /dev/tty routing is also why those messages leaked past go test's capture in your test run.

Summary: post-commit = 7-layer only, stderr+exit suppressed, no OPF output ever. pre-push = runs OPF, stderr visible, failures abort the push, and OPF progress is routed to /dev/tty so it shows up regardless.

Confirmed — and the missing piece resolves it cleanly. The post-commit path is gated by an explicit usePrivacyFilter bool, and it passes false:

And RedactBlobBytes routes that to the non-OPF functions (JSONLBytes/Bytes), never the *WithPrivacyFilter variants (committed.go:1944-1963). The doc comment says it outright:

Post-commit condensation uses false (fast path). The pre-push rewrite … uses true.

So even though EnsureRedactionConfigured does wire a live OPF runtime into every hook process (ConfigurePrivacyFilter → newShellOut(...), called from hooks_git_cmd.go:84 for all hooks), the post-commit path structurally can't reach the progress writes — it calls the 7-layer functions directly. The only production path that hits → scanning / ✓ done / × unavailable is the pre-push rewrite (BatchBytesWithPrivacyFilter).

So: yes, you're right

The stated rationale for /dev/tty is now obsolete:

That redirect only ever mattered if OPF ran under post-commit. It doesn't anymore. And pre-push is installed deliberately without 2>/dev/null (hooks.go:199, pre-push "$1"), so plain os.Stderr would be visible on the terminal exactly when we want it.

Switching opfStderr from openTTYOrDiscard() to os.Stderr would actually be a net improvement:

• pre-push: still visible (stderr isn't swallowed there).
• tests: the progress would go to go test's captured stderr instead of bleeding straight to the terminal — which incidentally fixes the leakage from your very first question, more cleanly than the per-test io.Discard override.

Two caveats before doing it

1. Edge case the /dev/tty trick still covers: if a future caller runs OPF (usePrivacyFilter=true) from a context whose stderr is redirected — e.g. someone adds an OPF pass back into post-commit, or a wrapper that swallows hook stderr. Today nothing does, so this is purely hypothetical.
2. Update the opfStderr comment (and arguably keep it as a var so tests can still override it), since its justification text would no longer be accurate.

Want me to make the switch (os.Stderr + comment update) and drop the now-redundant openTTYOrDiscard helper?

Done. Both packages pass, and notice the output is now clean — no → scanning… / ✓ done lines bleeding into the test run anymore, which was the symptom from your very first question.

What changed

In redact/opf.go:

• Replaced openTTYOrDiscard() (which opened /dev/tty) with plain os.Stderr.
• Removed the now-dead openTTYOrDiscard helper.
• Rewrote the comment to explain the current reality: OPF only runs in the pre-push rewrite, whose hook is installed without 2>/dev/null, so stderr is visible; post-commit never invokes OPF (it goes through RedactBlobBytes(..., usePrivacyFilter=false)), so the /dev/tty survival trick is obsolete.
• Kept opfStderr as an overridable var so tests can still redirect it.

Net effects

• pre-push (production): progress still shows on the terminal during git push — stderr isn't swallowed there.
• tests: the messages now route to go test's captured stderr instead of straight to /dev/tty, so they no longer interleave with PASS/ok lines. The strategy OPF tests that used configureFakeOPF (and never silenced output) are now quiet on success without needing a per-test io.Discard.

Verified: go build ./redact/..., mise run fmt, mise run lint (0 issues), and go test ./redact/... ./cmd/entire/cli/strategy/... all pass.

This is local to the redact/strategy area and unrelated to your clean working tree's prior state — want me to also sweep the redact tests that still set opfStderr = io.Discard (now redundant for the leakage reason, though still fine for tests that assert on emptiness), or leave them as-is?

Looking at the actual tests, let me give you a precise explanation — and correct a bit of imprecision in how I phrased that sentence.

What those tests do

Every one of them does the same three lines, e.g. redact/opf_test.go:133-135:

That swaps the package-global opfStderr to io.Discard (a writer that throws everything away) for the duration of the test, then restores it.

Why they did that — and why it's now "redundant for the leakage reason"

Before my change, opfStderr defaulted to /dev/tty. That's the special part: /dev/tty writes straight to the controlling terminal and bypasses go test's output capture. So any test that exercised an OPF path would splatter → scanning… / ✓ done onto the developer's terminal — exactly the leakage you noticed at the very start. These io.Discard overrides were the defense against that: "don't write to the terminal during this test."

After my change, opfStderr defaults to os.Stderr, which go test does capture (buffered per-test, shown only on failure or under -v). So the messages can no longer leak to the terminal on their own. That means the io.Discard overrides are no longer doing the job they were added for — hence "redundant for the leakage reason." Removing them ("sweeping" them) wouldn't reintroduce the leak.

Where I was imprecise — the "assert on emptiness" clause

That part of my sentence was loose, and checking the code, it doesn't actually apply here. To assert on output, a test would redirect opfStderr to a bytes.Buffer and then check what landed in it (either that a message appeared, or that the buffer stayed empty). None of these redact tests do that — they all use io.Discard, which keeps no record to assert against. So the caveat I tacked on describes a hypothetical pattern, not these tests. I shouldn't have implied it covered them.

So the honest bottom line

• The io.Discard overrides are now redundant and could be removed.
• They are also harmless to keep, and there's one mild reason to: under go test -v, captured stderr is printed even on passing tests, so the override keeps verbose runs quiet. As pure defensive isolation, that's fine.

My recommendation: leave them. They cost nothing, and they keep -v output clean. Removing ~11 three-line blocks is churn for no real benefit now that the leakage is fixed at the source. I'd only touch one if I were converting it to capture output for an assertion — and none currently need that.

All clean. Done.

Summary of the consistency cleanup

You were right — the 11 per-test io.Discard overrides were the only place doing that pattern, while the sibling strategy package already silences its OPF progress writer once in TestMain. I made redact follow the same convention:

• Added redact/global_test.go with a TestMain that sets opfStderr = io.Discard process-wide (mirrors strategy/global_test.go).
• Removed all 11 per-test blocks (8 in opf_test.go, 3 in batch_test.go) and the now-unused io imports from both files.
• Updated the configureFakeOPF helper comment in batch_test.go to point at the TestMain suppression instead of describing a redirect it no longer does.

Two side benefits beyond consistency:

• Race-free. The old per-test blocks mutated a package global under t.Parallel(); collapsing to a single set-once in TestMain removes that.
• Less churn per test. New OPF tests no longer need to remember the 3-line boilerplate.

Verified: mise run fmt, mise run lint (0 issues), go test ./redact/... and ./cmd/entire/cli/strategy/... all pass.

Note this is all test-only plumbing — the production change from the earlier turn (opfStderr = os.Stderr in redact/opf.go) is what actually fixed the terminal leakage; this round just makes the test suppression idiomatic.

Done. Draft PR opened: https://github.com/entireio/cli/pull/1470

• Branch: chore/opf-stderr-no-tty (pushed, tracking origin)
• Commit: 626a0344e — redact: route OPF progress to stderr instead of /dev/tty
• PR: draft, 4 files changed (+31 / −52)

The push triggered the pre-push hook (you can see it pushed entire/checkpoints/v1 to the checkpoint remote), and it succeeded. Lint and the relevant tests were green before pushing. Let me know if you want me to mark it ready for review or adjust the title/body.