Skip to content

refactor(settings): route SettingsDefaults diagnostics through emitDiagnostic#2981

Closed
darion-yaphet wants to merge 1 commit into
thedotmack:mainfrom
darion-yaphet:fix/console-to-logger
Closed

refactor(settings): route SettingsDefaults diagnostics through emitDiagnostic#2981
darion-yaphet wants to merge 1 commit into
thedotmack:mainfrom
darion-yaphet:fix/console-to-logger

Conversation

@darion-yaphet

Copy link
Copy Markdown
Contributor

Summary

SettingsDefaultsManager is a base-layer library module (runs under the worker daemon and hooks) that emitted its [SETTINGS] diagnostics via raw console.warn. Those lines can be swallowed by the Phase 2 hook stderr buffer (#2292).

Replace the 5 console.warn calls with emitDiagnostic (from hook-io):

Why emitDiagnostic, not logger

logger transitively depends on this module (logger → shared/paths → SettingsDefaultsManager). Importing logger here would invert the layering and risk a circular dependency at bootstrap. hook-io is a leaf module (type-only import), so there is no cycle.

Scope note

This was scoped deliberately. Of 546 console.* calls in src/, the vast majority are correct and intentionally left as-is:

  • worker-service.ts / ServerBetaService.ts — CLI entrypoints; console.log(JSON.stringify(...)) is the program's stdout output contract. Routing to the file logger would silence the CLI.
  • npx-cli/**, *Installer.ts, bin/** — user-facing CLI / install-time output.
  • ui/viewer/** — browser React code; the Node file-based logger cannot run there.
  • utils/logger.ts, shared/hook-io.ts — the logger's own stderr fallback.

Verification

  • tsc --noEmit clean
  • npm run build green
  • bun test — 39 pass / 0 fail (incl. settings-defaults-manager.test.ts and logger-usage-standards.test.ts)
  • [SETTINGS] diagnostics confirmed still printing to stderr in test output

SettingsDefaultsManager used raw console.warn for its [SETTINGS]
diagnostics. Replace with emitDiagnostic (hook-io) so the lines survive
the Phase 2 hook stderr buffer (thedotmack#2292) — the same channel logger uses —
instead of being swallowed by raw console.

Keeps the deliberate stderr-not-stdout behavior that protects the
machine-readable JSON contract on stdout. emitDiagnostic is chosen over
logger because logger transitively depends on this module via
shared/paths, so importing logger here would invert layering and risk a
circular dependency at bootstrap. hook-io is a leaf module (type-only
import), so no cycle.
@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces five console.warn calls in SettingsDefaultsManager with emitDiagnostic from hook-io.ts, ensuring [SETTINGS] diagnostic lines bypass the Phase 2 hook stderr buffer (#2292) instead of being silently dropped.

  • Buffer bypass: emitDiagnostic routes through pinnedBypassWrite (the real fd writer captured before the buffer was installed), so diagnostics now survive the hook stderr capture window. In non-hook contexts (worker daemon, CLI), pinnedBypassWrite is null and the function falls back transparently to process.stderr.write, preserving existing behavior.
  • Layering preserved: hook-io.ts has only an import type of cli/types.ts, so there is no runtime dependency that could form a cycle with logger → shared/paths → SettingsDefaultsManager.

Confidence Score: 5/5

Safe to merge; the change is a targeted swap of one output channel for another with no logic changes.

All five substitutions are mechanically correct: newlines are explicitly appended (bypassing the implicit newline that console.warn provides), message content is preserved exactly, and emitDiagnostic correctly falls back to process.stderr.write in non-hook contexts where pinnedBypassWrite is null. The hook-io import carries no runtime dependencies that could form a cycle. Existing tests cover stdout discipline and all settings-loading paths; they continue to pass.

No files require special attention.

Important Files Changed

Filename Overview
src/shared/SettingsDefaultsManager.ts Replaces 5 console.warn calls with emitDiagnostic; newlines are explicitly appended, message content is preserved, and the import of hook-io introduces no circular dependency.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Hook as Hook Invocation
    participant SDM as SettingsDefaultsManager
    participant HookIO as hook-io (emitDiagnostic)
    participant Buffer as Stderr Buffer
    participant RealStderr as Real stderr (fd)

    Hook->>Buffer: installHookStderrBuffer()
    Hook->>SDM: loadFromFile(path)
    SDM->>HookIO: emitDiagnostic("[SETTINGS] ...")
    Note over HookIO: pinnedBypassWrite != null
    HookIO->>RealStderr: bypassWrite(line)
    RealStderr-->>Hook: Diagnostic visible

    Note over Hook,RealStderr: Before this PR (console.warn path)
    SDM--xBuffer: console.warn captured by buffer
    Buffer--xRealStderr: drop() on success — diagnostic lost
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Hook as Hook Invocation
    participant SDM as SettingsDefaultsManager
    participant HookIO as hook-io (emitDiagnostic)
    participant Buffer as Stderr Buffer
    participant RealStderr as Real stderr (fd)

    Hook->>Buffer: installHookStderrBuffer()
    Hook->>SDM: loadFromFile(path)
    SDM->>HookIO: emitDiagnostic("[SETTINGS] ...")
    Note over HookIO: pinnedBypassWrite != null
    HookIO->>RealStderr: bypassWrite(line)
    RealStderr-->>Hook: Diagnostic visible

    Note over Hook,RealStderr: Before this PR (console.warn path)
    SDM--xBuffer: console.warn captured by buffer
    Buffer--xRealStderr: drop() on success — diagnostic lost
Loading

Reviews (1): Last reviewed commit: "refactor(settings): route diagnostics th..." | Re-trigger Greptile

@thedotmack

Copy link
Copy Markdown
Owner

Merged into the community-edge branch — claude-mem's bleeding-edge release line. Closing here since your change now lives on that branch. See the branch model (community-edge → core-dev → main): https://docs.claude-mem.ai/branches . Thanks for the contribution! 🙏

@thedotmack thedotmack closed this Jul 5, 2026
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.

2 participants