Skip to content

Route structured logs to a file instead of the console - #358

Merged
TheGreatAxios merged 2 commits into
mainfrom
cl-5593-log-sink-off-screen
Aug 7, 2026
Merged

Route structured logs to a file instead of the console#358
TheGreatAxios merged 2 commits into
mainfrom
cl-5593-log-sink-off-screen

Conversation

@TheGreatAxios

@TheGreatAxios TheGreatAxios commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • A raw structured log line from a vendored logger (interchange.inference.default-director) could reach stdout mid-frame and corrupt the TUI's alternate screen, because @intx/log installs a console sink as an import side effect and nothing in Corbits ever replaced it.
  • Add installFileLogSink (src/logging/sink.ts) and call it as the first statement of mainWithRunners, before loadConfig or anything else can log, so every logger — including ones inside vendor/ — writes to ~/.corbits/logs/corbits.log instead of the terminal.
  • The file sink's floor is debug, not warning: a file has no screen to corrupt, and a dozen existing logger.debug calls in src/tui/runner.ts and src/exec/runner.ts exist specifically to diagnose teardown races. A higher floor would have silently made those unreachable.
  • No change to error presentation: the readable, sentence-form error was already rendered elsewhere in the transcript; this only removes the duplicate raw log line that used to leak onto the screen.

Verification

  • bun run typecheck
  • bun run build
  • bun run test (3968 pass)
  • New test drives the real OpenTUI renderer (src/tui-opentui/log-sink.test.ts): mounts the shell, fires the vendored logger's exact call shape, and asserts the captured frame is unchanged, process.stdout.write/process.stderr.write are never called, and the record lands in the log file.
  • The riskiest property — that no module-scope getLogger call in the reachable import graph actually logs before installFileLogSink() runs — was verified by tracing the import graph rather than covered by an automated test; a mock-based ordering test was attempted and dropped after it deadlocked bun's module-mock cache. A subprocess test spawning the real entry point and asserting nothing reaches stdout would close this gap and is a reasonable follow-up.

Known follow-ups (not fixed here)

  • src/logging/sink.ts appends to the log file forever with no rotation or size cap.
  • The hand-rolled LogRecord type in src/logging/sink.ts structurally mirrors @logtape/logtape's real type (two packages away, not a declared dependency) rather than importing it. It typechecks today — the real type is strictly wider — but nothing catches drift if a future LogTape version renames or narrows a field this file reads.

Closes CL-5593

@linear-code

linear-code Bot commented Aug 7, 2026

Copy link
Copy Markdown

CL-5593

@intx/log installs a console sink as an import side effect, so a
vendored logger's error could land on stdout mid-frame while the TUI
held the alternate screen, corrupting the visible prompt box with a
raw JSON log record.

Install a file-backed sink as the first statement in mainWithRunners,
before config loads or any other subsystem can log, replacing the
console default entirely. The readable, human-facing error was
already rendered elsewhere in the transcript; this only stops the
duplicate raw log line from ever reaching the terminal.
Filtering the file sink at warning silently disabled a dozen
logger.debug calls written specifically to diagnose teardown races
in the TUI and exec runners. A file has no screen to corrupt, so
drop the floor to debug and let those diagnostics reach it.
@TheGreatAxios
TheGreatAxios force-pushed the cl-5593-log-sink-off-screen branch from b75737f to d1f9829 Compare August 7, 2026 07:37
@TheGreatAxios
TheGreatAxios merged commit 55fcd6f into main Aug 7, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant