refactor(debug): unify debug output through DebugTracer

[nextlevelshit]

Summary

Unifies debug output across Wave through a centralized DebugTracer mechanism. Resolves #537.

Changes

• Implements debug output unification pattern
• Replaces scattered debug logging with consistent DebugTracer interface
• Improves debuggability and observability

Testing

• All tests pass: go test -race ./...
• Branch rebased on main (commit $(git rev-parse --short HEAD))

[nextlevelshit]

Code Review (Wave Pipeline)

Overall Assessment: APPROVE

This PR is a clean, well-structured refactor that replaces 8 unstructured fmt.Fprintf(os.Stderr, "[DEBUG]...") calls with structured e.trace() calls routed through DebugTracer. It adds functional options, stderr mirroring with credential scrubbing, and new trace event type constants. No new external inputs, APIs, or authentication boundaries are introduced. Test coverage for the new functionality is solid, including concurrency safety verification.

No critical or high-severity issues were found across both security and quality review passes.

---

Critical Issues

None.

---

Issues to Address

1. Orphan temp file leaked by scrubber initialization (MEDIUM)

internal/audit/trace.go:81-88

NewDebugTracer creates a TraceLogger via NewTraceLoggerWithDir(os.TempDir()) solely to reuse its scrub() method, leaving an empty file in /tmp that is never cleaned up. Over many debug runs, these accumulate on disk and leak timing information about Wave debug sessions on shared systems.

Fix: Extract the credential regex compilation into a standalone helper (e.g., newCredentialScrubber()) that returns just the regex without creating a file. Alternatively, call os.Remove() on the scrubber's file path after closing it.

2. DebugVerbose() flag is dead code (MEDIUM)

internal/event/emitter.go:151, cmd/wave/commands/run.go:359

SetDebugVerbosity(true) is called when --debug is set, and the DebugVerbose() accessor exists, but nothing in the emitter or executor ever reads this flag to gate behavior. This is plumbing without a consumer.

Fix: Either wire DebugVerbose() into event emission to conditionally gate verbose events, or remove the flag until it has a consumer. Dead infrastructure creates confusion.

---

Suggested Improvements

3. trace() helper silently discards errors (MEDIUM)

internal/pipeline/executor.go:2394-2404

The trace() convenience method ignores the error returned by Emit(). While trace events shouldn't block execution, silently swallowing errors means disk-full conditions produce no diagnostic signal. Consider logging failures at debug level.

4. Non-deterministic metadata key ordering in stderr mirror (MEDIUM)

internal/audit/trace.go:155-157

formatTraceForStderr iterates over ev.Metadata with range, producing random key ordering. Sorting keys before iteration would make debug output deterministic and easier to diff across runs.

5. Path traversal via unsanitized runID (MEDIUM — pre-existing)

internal/audit/trace.go:72

The --run CLI flag allows arbitrary strings as runID, which flows into filepath.Join(traceDir, runID+".json"). A value like ../../etc/cron.d/evil would resolve outside the trace directory. This is pre-existing (not introduced by this PR), but worth a follow-up fix: validate filepath.Base(runID) == runID.

6. Metadata values with spaces break key=value format (LOW)

internal/audit/trace.go:156

Values containing spaces produce ambiguous output like error=file not found. Quoting values with spaces (%s=%q) would make stderr output parseable.

7. Inconsistent trace event type naming (LOW)

internal/pipeline/executor.go

New trace calls use defined constants (audit.TracePromptLoad, etc.), but pre-existing calls still use string literals ("adapter_start", "contract_validation_end", etc.). Defining constants for the remaining event types would prevent typos and enable IDE refactoring.

8. Missing test for WithStderrMirror(true) public API (LOW)

All stderr mirror tests use the internal withStderrWriter(&buf) helper. No test verifies that WithStderrMirror(true) actually sets stderrMirror to os.Stderr.

---

Positive Observations

• Well-scoped refactor: Consolidates dual debug systems (unstructured stderr + trace files) into a single structured tracer — net improvement in observability
• Credential scrubbing is correct: Applied to both file output and stderr mirror, verified by dedicated tests
• Concurrency safety verified: TestDebugTracer_ConcurrentEmit exercises 50 concurrent goroutines; all locking is correct
• Functional options pattern: Clean API design for NewDebugTracer configuration
• OWASP assessment clean: No injection vectors, appropriate file permissions (0755/0644), standard encoding/json marshaling
• Good test coverage: Stderr mirror, credential scrubbing in mirror, functional options, new event types, emit-after-close, and concurrency are all tested

---

Generated by Wave pr-review pipeline