Fix stderr display corruption in TUI by m-aebrer · Pull Request #213 · aebrer/dreb

m-aebrer · 2026-05-15T14:04:29Z

Closes #212

Fix STDERR messages corrupting the TUI by intercepting process.stderr.write in interactive mode, replacing all console.error calls with a structured logger, and displaying user-relevant warnings in the chat feed.

Implementation plan posted as a comment below.

m-aebrer · 2026-05-15T14:04:50Z

Implementation Plan

Problem Analysis

In interactive TUI mode, process.stderr is completely unguarded. The 68 console.error() calls across the codebase write raw bytes directly to terminal fd 2, corrupting the TUI's differential renderer. The highest-frequency offenders are in subagent.ts (fires on every spawn/close). Additionally, package-manager.ts inherits fd 2 for child process stdio, letting npm output bypass the TUI entirely.

Approach

Three-layer fix:

Stderr interception — Monkey-patch process.stderr.write during interactive mode to prevent raw terminal writes
Structured logging — Replace console.error calls with a logger that routes messages appropriately based on mode
TUI display — Show user-relevant stderr content in the chat feed using existing showWarning()/showStatus() patterns

Deliverables

1. packages/coding-agent/src/core/stderr-guard.ts (new file)

takeOverStderr(callback) — intercepts process.stderr.write, routes to callback instead of terminal
restoreStderr() — restores original behavior
isStderrTakenOver() — query state
writeRawStderr(text) — bypass for intentional writes (error reporting to user's shell)
Callback receives (message: string) for each intercepted write

2. packages/coding-agent/src/core/logger.ts (new file)

log.debug(msg) — suppressed in interactive mode, writes to stderr in non-interactive
log.warn(msg) — displayed to user in TUI feed, writes to stderr in non-interactive
log.error(msg) — displayed to user in TUI feed, writes to stderr in non-interactive
Mode-aware: auto-detects whether TUI is active and routes accordingly
Configurable: DREB_DEBUG=1 env var shows debug messages in TUI too

3. Modify packages/coding-agent/src/modes/interactive/interactive-mode.ts

Call takeOverStderr() at TUI init, pass callback that buffers messages
Process buffered messages: display user-visible ones via showWarning()/showStatus()
Filter debug noise (messages prefixed with [subagent], [debug], etc.) unless DREB_DEBUG=1
Call restoreStderr() on TUI teardown
Add a debounced requestRender(true) safety net if any stderr write somehow leaks

4. Modify packages/coding-agent/src/core/tools/subagent.ts

Replace all 17 console.error() calls with log.debug() or log.warn() as appropriate:
- Lines 200, 362-363 (spawn/close): log.debug() — diagnostic noise
- Lines 687, 701, 706 (model fallback probing): log.debug()
- Line 716 (model warning): log.warn()
- Lines 116, 121, 128 (agent file parse errors): log.warn()
- Lines 290, 296, 310 (stream/parse errors): log.warn()
- Lines 439, 443 (session file info): log.debug()
- Lines 1287, 1318, 1338 (completion errors): log.warn()

5. Modify packages/coding-agent/src/main.ts

Replace console.error calls with log.warn() / log.error() as appropriate
Extension load failures: log.warn()
Fatal startup errors: log.error() (fire before TUI is up, use real stderr naturally)

6. Modify packages/coding-agent/src/core/package-manager.ts

Change stdio: ["ignore", 2, 2] to pipe stderr when TUI is active
Route piped stderr through the logger

7. Modify remaining console.error call sites (across web.ts, web-search-queue.ts, terminal-render.ts, model-resolver.ts, print-mode.ts, timings.ts)

Categorize each as log.debug(), log.warn(), or log.error()

Files to Create/Modify

File	Action
`packages/coding-agent/src/core/stderr-guard.ts`	Create — stderr interception
`packages/coding-agent/src/core/logger.ts`	Create — structured logging
`packages/coding-agent/src/modes/interactive/interactive-mode.ts`	Modify — wire up stderr guard + display
`packages/coding-agent/src/core/tools/subagent.ts`	Modify — replace console.error with logger
`packages/coding-agent/src/main.ts`	Modify — replace console.error with logger
`packages/coding-agent/src/core/package-manager.ts`	Modify — pipe stderr instead of fd 2 inherit
`packages/coding-agent/src/core/tools/web.ts`	Modify — use logger
`packages/coding-agent/src/core/tools/web-search-queue.ts`	Modify — use logger
`packages/coding-agent/src/core/tools/terminal-render.ts`	Modify — use logger
`packages/coding-agent/src/core/model-resolver.ts`	Modify — use logger
`packages/coding-agent/src/modes/print-mode.ts`	Modify — use logger
`packages/coding-agent/src/core/timings.ts`	Modify — use logger
`packages/coding-agent/test/stderr-guard.test.ts`	Create — unit tests
`packages/coding-agent/test/logger.test.ts`	Create — unit tests

Testing Approach

Unit tests (packages/coding-agent/test/stderr-guard.test.ts):

takeOverStderr() intercepts writes, callback receives content
restoreStderr() returns to normal behavior
writeRawStderr() bypasses interception
Multiple calls to takeOverStderr() are idempotent
Intercepted writes don't reach the real terminal (spy on original write)

Unit tests (packages/coding-agent/test/logger.test.ts):

log.debug() suppressed when stderr is taken over (no callback fire) unless DREB_DEBUG=1
log.warn() fires callback when stderr is taken over
log.error() fires callback when stderr is taken over
All levels write to real stderr when not taken over
DREB_DEBUG=1 makes debug messages visible

Integration test (extend existing packages/coding-agent/test/stdout-cleanliness.test.ts):

Verify no raw stderr leaks during an interactive mode lifecycle

Acceptance Criteria

No console.error calls remain in production code — all replaced with log.debug/warn/error
In interactive TUI mode, stderr writes never reach the raw terminal
User-visible warnings (model fallback, extension errors, parse failures) appear in the chat feed styled with theme.fg("warning", ...)
Debug messages ([subagent] spawn:, [subagent] close:) are suppressed unless DREB_DEBUG=1
Non-interactive modes (JSON, RPC, print) behave exactly as before — stderr remains the diagnostic channel
The "Working" spinner correctly clears after agent turns complete (symptom resolves)
All new code has tests; all existing tests pass

Risks

Third-party libraries writing to stderr — Our interception catches process.stderr.write calls from JS, but native addons calling write(2, ...) directly bypass Node. The requestRender(true) safety net handles this.
Child process fd 2 inheritance — Processes spawned with explicit fd 2 write directly to the kernel fd, bypassing our patch. Need to audit all spawn sites (most already pipe stderr).
Extension editor — extension-editor.ts uses stdio: "inherit" for editors (vim, etc.) which need the real terminal. This is fine — the TUI is suspended during editor use and does a requestRender(true) on return.
Performance — Buffering stderr adds negligible overhead (sub-microsecond per write vs 16ms render loop).

Plan created by mach6

Intercept process.stderr.write in interactive mode to prevent raw writes from corrupting the TUI's differential renderer. Replace all console.error calls with a structured logger (log.debug/warn/error) that routes messages through the TUI feed when active, and to real stderr in non-interactive modes. - Add stderr-guard.ts: takeOverStderr/restoreStderr lifecycle - Add logger.ts: mode-aware structured logging with DREB_DEBUG support - Wire stderr guard into interactive mode (init, suspend, editor, shutdown) - Migrate all 68 console.error calls across 15 source files - Fix package-manager.ts to pipe stderr when TUI is active - Add 15 unit tests for stderr-guard and logger - Update 5 test files to spy on log.* instead of console.error

m-aebrer · 2026-05-15T14:46:10Z

Progress Update

Implementation complete. All plan deliverables addressed:

New files

stderr-guard.ts — Intercepts process.stderr.write in interactive mode, routes all writes through a callback instead of hitting the terminal raw
logger.ts — Structured logger (log.debug/warn/error) that auto-routes based on whether TUI is active. DREB_DEBUG=1 shows debug messages in TUI.

TUI integration

interactive-mode.ts — Calls takeOverStderr() after ui.start(), restoreStderr() on suspend/editor/shutdown. Intercepted messages displayed via showWarning() in the chat feed.

console.error migration (68 calls across 15 files)

subagent.ts — 17 calls: spawn/close/model probing → log.debug, errors → log.warn/error
main.ts — 24 calls: CLI errors → log.error, non-fatal warnings → log.warn
package-manager.ts — Now pipes stderr when TUI is active instead of inheriting fd 2
Remaining files: web.ts, web-search-queue.ts, terminal-render.ts, model-resolver.ts, print-mode.ts, timings.ts, event-bus.ts, agent-session.ts, args.ts, file-processor.ts, buddy-controller.ts

Tests

15 new unit tests (stderr-guard: 8, logger: 7)
5 existing test files updated to spy on log.* instead of console.error
All 3125 tests pass, 0 failures

Commit: 3a227c2

Progress tracked by mach6

m-aebrer · 2026-05-15T14:59:38Z

Code Review

Critical

Finding 1: Unguarded callback — exceptions propagate out of process.stderr.write

File: packages/coding-agent/src/core/stderr-guard.ts (lines 38–42)
Confidence: 82 | Agent: error-auditor
If the showWarning callback throws (e.g., TUI component in unexpected state), the exception exits through the patched process.stderr.write. Node internals that call stderr.write never expect it to throw — this crashes the process. The write completion callback cb(null) is also never reached, breaking backpressure signalling.

Important

Finding 2: String(chunk) on raw Uint8Array produces comma-delimited decimal garbage

File: packages/coding-agent/src/core/stderr-guard.ts (line 38)
Confidence: 85 | Agent: error-auditor
Buffer overrides toString() to decode UTF-8, but a plain Uint8Array inherits TypedArray.prototype.toString which is join(','). A raw Uint8Array passed to process.stderr.write produces "104,101,108,108,111" displayed as a TUI warning. Fix: typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString()

Finding 3: log.error messages displayed as "Warning:" in TUI, losing severity distinction

File: packages/coding-agent/src/core/logger.ts + interactive-mode.ts
Confidence: 87 | Agent: code-reviewer
Both log.warn and log.error route through the same writeToStderr → process.stderr.write → callback → showWarning(). By the time bytes reach the interceptor, severity info is gone. Critical failures like "[subagent] CRITICAL: Last-resort notification failed" render as yellow warnings.

Finding 4: Model fallback probing messages classified as log.debug() — invisible to users

File: packages/coding-agent/src/core/tools/subagent.ts (lines 688, 702)
Confidence: 90 | Agent: manual observation
Messages like "Model X unavailable... Trying next fallback..." are suppressed in TUI mode. Users can't see in real-time that their preferred model failed and a weaker fallback is being tried. The info only appears post-hoc in tool results, when the work is already done and can't be cancelled. The issue explicitly asks for stderr content to be "displayed properly" — these are user-relevant degradation warnings, not debug noise.

Finding 5: Triplicate takeOverStderr callback pattern

File: packages/coding-agent/src/modes/interactive/interactive-mode.ts (lines 592, 2976, 3159)
Confidence: 97 | Agent: simplifier
The identical callback body appears in init(), SIGCONT handler, and external-editor resume. Extract to a private method — any future routing change (e.g., severity prefixing from finding 3) must otherwise be replicated in three places.

Suggestions

Finding 6: exit event fires before piped stderr is fully drained

File: packages/coding-agent/src/core/package-manager.ts
Confidence: 85 | Agent: error-auditor
In TUI mode, subprocess exit resolves/rejects the promise before pipe buffers are drained. Warnings from npm/git appear after the error in the chat feed. Use close event instead of exit.

Finding 7: Trailing-newline regex only strips one \n

File: packages/coding-agent/src/modes/interactive/interactive-mode.ts (3 locations)
Confidence: 82 | Agent: code-reviewer
/\n$/ removes exactly one trailing newline. Chunks ending in \n\n (git/npm separator lines) retain extra whitespace in warning bubbles. Use /\n+$/ or /[\r\n]+$/.

Finding 8: Package-manager TUI stdio routing is untested

File: packages/coding-agent/src/core/package-manager.ts
Confidence: 93 | Agent: test-reviewer
The new isStderrTakenOver() branch with piped stdio has no test coverage.

Finding 9: args.ts warning paths via log.warn have no coverage

File: packages/coding-agent/src/cli/args.ts
Confidence: 88 | Agent: test-reviewer
Unknown tool names and invalid thinking levels emit log.warn but no test asserts this.

Finding 10: Stale console.error suppressor in test

File: packages/coding-agent/test/subagent-model-fallback.test.ts
Confidence: 82 | Agent: test-reviewer
vi.spyOn(console, "error") remains but there are no more console.error calls to intercept.

Finding 11: log.debug reimplements routing logic already in writeToStderr

File: packages/coding-agent/src/core/logger.ts (lines 38–46)
Confidence: 88 | Agent: simplifier
Can simplify to if (!isStderrTakenOver() || isDebugEnabled()) writeToStderr(message);

Finding 12: AC6 (spinner clears after agent turns) — no implementation

File: packages/coding-agent/src/modes/interactive/interactive-mode.ts
Confidence: 90 | Agent: completeness-checker
Plan acceptance criteria item 6 about the "Working" spinner is not addressed in code. If it was already working, should be documented/tested.

Finding 13: console.error calls remain in packages/ai

File: packages/ai/src/utils/oauth/openai-codex.ts (6 calls)
Confidence: 82 | Agent: completeness-checker
AC1 says "no console.error in production code." The stderr guard catches these at the process level so the TUI won't be corrupted, but the letter of AC1 is unmet.

Strengths

Clean separation of concerns: stderr-guard handles interception, logger handles routing, interactive-mode handles display
Idempotent design of takeOverStderr prevents double-patching bugs
Correct restore/re-intercept around suspend and external-editor flows
Comprehensive migration of 68 console.error calls across 15 source files
Good test coverage for the new modules (15 tests total)
Analogous design to the existing output-guard.ts (stdout protection) makes the codebase internally consistent

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier

Reviewed by mach6

m-aebrer · 2026-05-15T15:03:16Z

Review Assessment

#213 (comment)

Classifications

Finding	Classification	Reasoning
1: Unguarded callback in stderr.write	Genuine issue	Confirmed: no try-catch around `stderrTakeoverState?.callback(text)`. If showWarning throws, exception propagates into whatever Node internal called stderr.write — potential process crash.
2: `String(chunk)` on Uint8Array produces garbage	Genuine issue	Confirmed: `String(new Uint8Array([104,101,108,108,111]))` → `"104,101,108,108,111"`. Native addons may write raw Uint8Array to stderr per Node's API.
3: `log.error` displayed as "Warning:" in TUI	Genuine issue	Confirmed: no severity branching in the interception path. `log.error("CRITICAL...")` renders as yellow "Warning:" which misrepresents severity.
4: Model fallback probing messages invisible to users	Genuine issue	The plan classified these as debug, but per issue author's direct feedback: users need to see model degradation in real-time so they can cancel before work completes on an unwanted fallback model. These should be `log.warn()`.
5: Triplicate takeOverStderr callback	Nitpick	Confirmed identical in 3 places. Extracting to a method is cleaner but not a correctness issue.
6: `exit` fires before stderr drains	Deferred	Real bug (should use `close` event) but pre-existing code not introduced by this PR.
7: Trailing-newline regex strips only one	Nitpick	`/\n$/` is fine for typical single-line writes. Multi-newline edge case is cosmetic.
8: Package-manager stdio routing untested	Nitpick	Branch is straightforward pipe + data handlers; underlying logger is tested.
9: args.ts warning paths untested	Nitpick	Change was just console.error → log.warn; validation logic itself unchanged.
10: Stale console.error spy in test	Nitpick	Dead code but harmless.
11: log.debug reimplements writeToStderr	False positive	Intentionally different behavior (suppression gate). Current code is clear and correct.
12: AC6 spinner — no implementation	False positive	AC6 describes an expected consequence of fixing stderr corruption, not a separate feature. Spinner already cleared correctly — it was stderr writes visually corrupting the screen.
13: console.error in packages/ai	Deferred	Different package, needs own logger or shared logging abstraction. The stderr guard catches these at process level so TUI isn't corrupted.

Action Plan

Wrap callback in try-catch (finding 1) — In stderr-guard.ts, catch exceptions from the callback and fall back to rawStderrWrite. Prevents process crashes from TUI rendering errors propagating through Node internals.
Handle Uint8Array chunks correctly (finding 2) — Replace String(chunk) with typeof chunk === "string" ? chunk : Buffer.from(chunk).toString(). Prevents garbled output from native addons.
Add severity awareness to TUI display (finding 3) — Either pass severity through a side channel (e.g., prefix stripped by callback), add a showError() path, or use the existing logger to tag messages before interception.
Reclassify model fallback probes as log.warn() (finding 4) — Change "Model X unavailable... Trying next fallback..." and "Model X failed probe..." from log.debug() to log.warn() so users see degradation in real-time. Keep "Using model X for subagent" as log.debug() (confirmation, not actionable).

Assessment by mach6

… fallback visibility

m-aebrer · 2026-05-15T15:17:55Z

Progress Update

Addressed 4 review findings:

Finding 1: Unguarded callback

Wrapped callback(text) in try-catch in stderr-guard.ts
On exception, falls back to rawStderrWrite(text) so the message at least reaches the terminal

Finding 2: Uint8Array decoding

Replaced String(chunk) with typeof chunk === "string" ? chunk : Buffer.from(chunk).toString()
Prevents garbled output from native addons or code that writes raw Uint8Array to stderr

Finding 3: Error/warning severity distinction

Added writeIntercepted(message, level) to stderr-guard.ts — logger calls it directly, bypassing process.stderr.write, with level info
Updated StderrCallback type to accept optional level parameter
Interactive mode callback routes level === "error" → showError() (red), otherwise → showWarning() (yellow)
Extracted triplicated callback into private activateStderrGuard() method

Finding 4: Model fallback probes now visible

Changed "Model X unavailable... Trying next fallback..." from log.debug() to log.warn()
Changed "Model X failed probe... Trying next fallback..." from log.debug() to log.warn()
Users now see model degradation in real-time and can cancel if needed
Kept "Using model X for subagent" as log.debug() (confirmation, not actionable)

Tests

6 new tests in stderr-guard.test.ts (callback-throws fallback, Uint8Array, Buffer, writeIntercepted level, writeIntercepted fallback, writeIntercepted without takeover)
Updated logger.test.ts to verify level passing
Updated subagent-model-fallback.test.ts to assert on log.warn for probe failures
Updated interactive-mode-suspend.test.ts for new activateStderrGuard method
All 3132 tests pass, 0 failures

Commit: 9f742ce

Progress tracked by mach6

m-aebrer · 2026-05-15T15:35:07Z

Code Review (Round 2)

Critical

No critical findings.

Important

Finding 1: activateStderrGuard routing logic is entirely untested

File: packages/coding-agent/src/modes/interactive/interactive-mode.ts (activateStderrGuard())
Confidence: 95 | Agent: test-reviewer
The callback that routes intercepted stderr into the TUI chat feed is the critical wiring this PR ships, yet none of its logic is tested. interactive-mode-suspend.test.ts only stubs activateStderrGuard: vi.fn() and asserts it's called — it never exercises the callback body. Untested behaviors: level === "error" routes to showError; level === "warn" / undefined routes to showWarning; trailing \n is stripped; empty-after-trim messages are suppressed.

Finding 2: runCommand stdio piping branch in package-manager.ts is untested

File: packages/coding-agent/src/core/package-manager.ts (runCommand)
Confidence: 92 | Agent: test-reviewer
All existing tests mock runCommand entirely via vi.spyOn(...).mockResolvedValue(undefined). The three stdio branches (TUI piped, stdout-taken fd2, inherit) are never exercised, and neither is the log routing of subprocess output. If the TUI-mode branch were broken, npm/git output would still corrupt the TUI — exactly the bug this PR fixes.

Finding 3: exit event fires before piped stderr is fully drained in runCommandCapture

File: packages/coding-agent/src/core/package-manager.ts (runCommandCapture, line ~2145)
Confidence: 83 | Agent: error-auditor
child.on("exit", ...) fires before all buffered pipe data has been delivered to data handlers. The rejection message is constructed with incomplete stderr — the missing bytes arrive on the next event-loop tick, too late. Most visible for verbose failures (git auth errors, npm resolver conflicts) where the last line(s) — often the most actionable — are silently absent. Fix: use close event instead of exit.

Finding 4: exit vs close in runCommand (TUI mode) — diagnostic warnings after error summary

File: packages/coding-agent/src/core/package-manager.ts (runCommand, line ~2195)
Confidence: 82 | Agent: error-auditor
Same exit-before-drain issue. In TUI mode, the promise rejects with "failed with code N" before stderr data events have been delivered. The user sees the error headline before the explanation: Error: npm install failed with code 1 appears first, then Warning: npm error code ENOENT appears after. Fix: switch to close event.

Suggestions

Finding 5: packages/ai console.error calls still present

File: packages/ai/src/utils/oauth/openai-codex.ts (6 calls)
Confidence: 85 | Agent: completeness-checker
Plan AC 1 says "no console.error in production code." These calls are caught by the stderr guard so TUI corruption is prevented, but they arrive at the callback with level === undefined, so OAuth errors like "Token refresh failed: 401" render as yellow warnings instead of red errors. packages/ai can't import the logger from packages/coding-agent without a circular dependency. packages/ai/src/cli.ts has 7 more, but that's a standalone CLI that never runs during TUI operation.

Finding 6: log.warn and log.error are structurally identical — extract shared helper

File: packages/coding-agent/src/core/logger.ts (lines 36–55)
Confidence: 85 | Agent: simplifier
The two methods are identical aside from the level string. Extract a shared writeAtLevel(message, level) helper. debug can't share it (it has the extra isDebugEnabled() guard), but warn and error can.

Finding 7: Unnecessary intermediate variable in activateStderrGuard

File: packages/coding-agent/src/modes/interactive/interactive-mode.ts (lines 3164–3179)
Confidence: 85 | Agent: simplifier
callback is used exactly once and immediately passed to takeOverStderr. Inline the lambda directly into the call. The explicit : StderrCallback annotation is also redundant — TypeScript infers parameter types from takeOverStderr's signature.

Finding 8: Inline dynamic import for StdioOptions type

File: packages/coding-agent/src/core/package-manager.ts (line 2167)
Confidence: 80 | Agent: simplifier
import("node:child_process").StdioOptions is a valid but unusual pattern — node:child_process is already imported at the top of this file. Pull StdioOptions into the existing top-level import for readability.

Finding 9: AC 6 (spinner clearing) has no implementation trace

Confidence: 80 | Agent: completeness-checker
Neither commit contains spinner or loading animation changes. If the symptom was caused entirely by stderr corruption (and thus fixed implicitly), a note in the PR confirming validation would close the loop.

Strengths

Clean separation of concerns: stderr-guard handles interception, logger handles routing, interactive-mode handles display
Correct ordering throughout: restoreStderr() before ui.stop(), activateStderrGuard() after ui.start()
Try-catch fallback to rawStderrWrite on callback throws prevents process crashes
Correct Uint8Array handling via Buffer.from(chunk).toString()
Idempotent takeOverStderr design prevents double-patching
Proper suspend/resume/editor lifecycle management
No memory leaks: child process stream listeners cleaned up automatically
Comprehensive unit test coverage for stderr-guard.ts (14 tests) and logger.ts (7 tests)
Code-reviewer found zero correctness issues in the implementation

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier

Reviewed by mach6

m-aebrer · 2026-05-15T15:38:34Z

Review Assessment

#213 (comment)

Classifications

Finding	Classification	Reasoning
1: `activateStderrGuard` routing logic untested	Genuine issue	The callback at line 3164 has real conditionals (level routing, newline trim, empty suppression) introduced by this PR. Underlying modules are well-tested but this integration wiring is not.
2: `runCommand` stdio piping branch untested	Nitpick	Straightforward pass-through. Testing requires spawning real processes or complex spawn mocking. The important behavior (log.warn routing) is already tested in logger.test.ts. Low value-to-effort ratio.
3: `exit` before drain in `runCommandCapture`	False positive	`runCommandCapture` was NOT modified by this PR. Pre-existing code outside PR scope.
4: `exit` vs `close` in `runCommand` (TUI mode)	Genuine issue	This IS new code. The promise rejects on `exit` while piped `data` events may still be pending. When a command fails, the final stderr chunk explaining WHY may arrive after reject, causing the error message to appear after the caller has already logged a generic failure. Should use `close`.
5: `packages/ai` console.error calls	Deferred	Different package, caught at process level by stderr guard (no TUI corruption). Rendering as warnings instead of errors is cosmetic. Needs a shared logging solution — not blocking.
6: Extract shared helper for log.warn/log.error	Nitpick	Each method is 5 lines, clearly named, immediately readable. Extracting a helper for two methods that differ by one string literal adds indirection without meaningful DRY benefit.
7: Unnecessary intermediate `callback` variable	Nitpick	The named variable with explicit type annotation makes the code self-documenting. Style preference with no correctness impact.
8: Inline dynamic import for `StdioOptions` type	Nitpick	Valid TypeScript. Pulling into top-level import is marginally cleaner but purely cosmetic.
9: AC 6 (spinner clearing) no implementation	False positive	Already assessed in round 1: AC6 describes an expected consequence of fixing stderr corruption, not a separate feature. The issue body confirms the problem was visual corruption, not broken spinner logic.

Action Plan

Use close event instead of exit in runCommand (finding 4) — Change child.on("exit", ...) to child.on("close", ...) so all piped stderr data is delivered before the promise resolves/rejects. Prevents the race where an error rejection fires before explanatory stderr output is displayed.
Add unit test for activateStderrGuard routing logic (finding 1) — Test the callback directly: verify level "error" routes to showError, other levels route to showWarning, trailing newlines are trimmed, and empty-after-trim messages are suppressed.

Assessment by mach6

…g tests

m-aebrer · 2026-05-15T17:05:57Z

Progress Update

Addressed round 2 review findings 1 and 4:

Finding 4: exit-before-drain race in `runCommand`

Changed child.on("exit", ...) to child.on("close", ...) in package-manager.ts
Ensures all piped stderr data is delivered before the promise resolves/rejects
Prevents the race where an error rejection fires before explanatory stderr output is displayed in TUI mode

Finding 1: `activateStderrGuard` routing untested

Added test/activate-stderr-guard.test.ts with 7 unit tests
Covers: level "error" → showError, level "warn"/undefined/debug → showWarning, trailing newline trim, empty suppression
Uses vi.mock to capture the callback installed by activateStderrGuard and tests it directly

Tests

All 3139 tests pass, 0 failures
Build succeeds, biome clean

Commit: bbad6aa

Progress tracked by mach6

chore: open PR for issue 212

f463249

m-aebrer marked this pull request as ready for review May 15, 2026 14:50

Address review findings: stderr-guard safety, severity routing, model…

9f742ce

… fallback visibility

Fix runCommand exit-before-drain race, add activateStderrGuard routin…

bbad6aa

…g tests

chore: bump version to 2.19.1, document DREB_DEBUG env var

ae1b21b

m-aebrer merged commit 1b9e739 into master May 15, 2026
2 checks passed

m-aebrer deleted the feature/issue-212-stderr-tui-display branch May 15, 2026 17:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix stderr display corruption in TUI#213

Fix stderr display corruption in TUI#213
m-aebrer merged 5 commits into
masterfrom
feature/issue-212-stderr-tui-display

m-aebrer commented May 15, 2026

Uh oh!

m-aebrer commented May 15, 2026

Uh oh!

m-aebrer commented May 15, 2026

Uh oh!

m-aebrer commented May 15, 2026

Uh oh!

m-aebrer commented May 15, 2026

Uh oh!

m-aebrer commented May 15, 2026

Uh oh!

m-aebrer commented May 15, 2026

Uh oh!

m-aebrer commented May 15, 2026

Uh oh!

m-aebrer commented May 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

m-aebrer commented May 15, 2026

Uh oh!

m-aebrer commented May 15, 2026

Implementation Plan

Problem Analysis

Approach

Deliverables

Files to Create/Modify

Testing Approach

Acceptance Criteria

Risks

Uh oh!

m-aebrer commented May 15, 2026

Progress Update

New files

TUI integration

console.error migration (68 calls across 15 files)

Tests

Uh oh!

m-aebrer commented May 15, 2026

Code Review

Critical

Important

Suggestions

Strengths

Uh oh!

m-aebrer commented May 15, 2026

Review Assessment

Classifications

Action Plan

Uh oh!

m-aebrer commented May 15, 2026

Progress Update

Finding 1: Unguarded callback

Finding 2: Uint8Array decoding

Finding 3: Error/warning severity distinction

Finding 4: Model fallback probes now visible

Tests

Uh oh!

m-aebrer commented May 15, 2026

Code Review (Round 2)

Critical

Important

Suggestions

Strengths

Uh oh!

m-aebrer commented May 15, 2026

Review Assessment

Classifications

Action Plan

Uh oh!

m-aebrer commented May 15, 2026

Progress Update

Finding 4: exit-before-drain race in runCommand

Finding 1: activateStderrGuard routing untested

Tests

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Finding 4: exit-before-drain race in `runCommand`

Finding 1: `activateStderrGuard` routing untested