Skip to content

fix(cli): survive unavailable chat log directory - #1099

Open
c8dhjp4tyv-bit wants to merge 2 commits into
CodebuffAI:mainfrom
c8dhjp4tyv-bit:fix/cli-logger-startup-resilience
Open

fix(cli): survive unavailable chat log directory#1099
c8dhjp4tyv-bit wants to merge 2 commits into
CodebuffAI:mainfrom
c8dhjp4tyv-bit:fix/cli-logger-startup-resilience

Conversation

@c8dhjp4tyv-bit

Copy link
Copy Markdown

Summary

  • prevent logger initialization from crashing the CLI when the current chat directory cannot be created
  • keep file logging best-effort while preserving the existing development and production destinations
  • add regression coverage for both destinations and the unavailable-chat-directory path

Fixes #783

Validation

  • logger.test.ts: 3 passed
  • common typecheck: passed
  • SDK typecheck: passed
  • git diff --check: passed
  • full CLI test command: 2,111 passed; existing checkout/environment failures remain in unrelated Infisical, read-only config, and missing release-dependency tests

@c8dhjp4tyv-bit
c8dhjp4tyv-bit force-pushed the fix/cli-logger-startup-resilience branch from fa4bc28 to b00d9e9 Compare August 23, 2026 21:28
@codebuff-team

Copy link
Copy Markdown
Contributor

Good, focused fix. The root cause is real: sendAnalyticsAndLog previously called getCurrentChatDir() and setLogPath() unguarded, so any filesystem failure (read-only config dir, permission denied, etc.) during logger setup would throw and take down the CLI, per #783.

The fix extracts the destination-resolution logic into a pure, testable function (resolveLogTarget) and wraps both the resolution and the actual setLogPath call in try/catch, correctly treating file logging as best-effort. The behavior for the dev/production destination selection is preserved unchanged (IS_DEV branch never touches the filesystem), so no regression there.

logger.test.ts covers the three relevant paths: dev target, production target, and the failure case where getCurrentChatDir throws — a sensible minimal test set for this change.

One thing worth double-checking before porting: does getCurrentChatDir() have any side effects that partially succeed before throwing (e.g., creating a directory then failing on a nested write)? If so it might be worth logging the swallowed error somewhere (even to stderr) for diagnosability, since silently disabling file logging could make future issues harder to debug. Not a blocker, just a suggestion.

Overall this is a small, well-scoped, well-tested fix that addresses a genuine crash path — good candidate for porting.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Aug 24, 2026

Copy link
Copy Markdown
Author

Checked the diagnosability point. getCurrentChatDir() only derives the project chat path and performs a recursive mkdirSync; there is no later state mutation that can partially succeed and make the logger inconsistent. Any mkdir/path failure is exactly the failure this PR now contains.

I intentionally left the fallback silent rather than writing to stderr: this path runs during normal TUI logging, so an EACCES warning on stderr can corrupt the terminal UI while the whole point of the change is to make file logging best-effort. The dev and production destinations remain unchanged, and logger setup failure no longer blocks CLI startup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't launch (ENOENT)

2 participants