Skip to content

fix: use config working directory for OpenAI logger path resolution in ACP mode#2675

Merged
Mingholy merged 1 commit intoQwenLM:mainfrom
LaZzyMan:fix-issue-2671
Mar 27, 2026
Merged

fix: use config working directory for OpenAI logger path resolution in ACP mode#2675
Mingholy merged 1 commit intoQwenLM:mainfrom
LaZzyMan:fix-issue-2671

Conversation

@LaZzyMan
Copy link
Copy Markdown
Collaborator

TLDR

Fix OpenAI logger failing to initialize in ACP mode (e.g., Zed editor) because process.cwd() returns / (filesystem root), causing mkdir '/logs' to fail with ENOENT.

Adds an optional cwd parameter to OpenAILogger constructor and passes config.getWorkingDir() from LoggingContentGenerator, so log directories are resolved relative to the project working directory instead of process.cwd().

Screenshots / Video Demo

N/A — no user-facing change in interactive CLI. This fixes an internal path resolution issue only observable in ACP mode.

Dive Deeper

Root Cause

In ACP mode (e.g., when launched by Zed editor), process.cwd() may return / (the filesystem root). The OpenAILogger constructor previously used process.cwd() to build the default log directory path (logs/openai), which resolved to /logs/openai. On macOS, creating directories at the filesystem root fails with ENOENT.

Fix

  • OpenAILogger (packages/core/src/utils/openaiLogger.ts): Added an optional cwd parameter to the constructor. When provided, it is used instead of process.cwd() for resolving the default log directory and relative custom paths. Falls back to process.cwd() when cwd is not provided (backward compatible).

  • LoggingContentGenerator (packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts): Passes config.getWorkingDir() as the cwd parameter when constructing OpenAILogger, ensuring the correct project directory is used.

Changes Summary

File Change
packages/core/src/utils/openaiLogger.ts Added optional cwd param to constructor
packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts Pass config.getWorkingDir() to OpenAILogger
packages/core/src/utils/openaiLogger.test.ts Added 5 test cases for cwd parameter behavior
packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts Added getWorkingDir to config mock

Reviewer Test Plan

  1. Enable OpenAI logging in settings (enableOpenAILogging: true)
  2. Run Qwen Code in ACP mode (e.g., via Zed editor)
  3. Verify no Failed to initialize OpenAI logger error occurs
  4. Verify logs are written under the project's logs/openai/ directory
  5. Run unit tests: npx vitest run packages/core/src/utils/openaiLogger.test.ts packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts

Testing Matrix

🍏 🪟 🐧
npm run
npx
Docker
Podman - -
Seatbelt - -

Linked issues / bugs

Fixes #2671

…n ACP mode

In ACP mode (e.g., Zed editor), process.cwd() may return '/' (filesystem root),
causing OpenAILogger to attempt creating '/logs/openai' which fails with ENOENT.

Add an optional 'cwd' parameter to OpenAILogger constructor and pass
config.getWorkingDir() from LoggingContentGenerator so that log directories
are resolved relative to the project working directory instead of process.cwd().

Fixes QwenLM#2671
@github-actions
Copy link
Copy Markdown
Contributor

📋 Review Summary

This PR addresses a critical bug where OpenAILogger fails to initialize in ACP mode (e.g., Zed editor) because process.cwd() returns / (filesystem root), causing directory creation to fail with ENOENT. The fix introduces an optional cwd parameter to OpenAILogger and passes config.getWorkingDir() from LoggingContentGenerator, ensuring log directories resolve relative to the project working directory. The implementation is clean, well-tested, and maintains backward compatibility.

🔍 General Feedback

  • Clean, minimal fix: The change is surgical and focused on the root cause without unnecessary refactoring
  • Good backward compatibility: The optional cwd parameter with fallback to process.cwd() ensures existing code continues to work
  • Comprehensive test coverage: 5 new test cases cover all edge cases for the cwd parameter behavior
  • Good documentation: JSDoc comments clearly explain the purpose and usage of the new parameter, including the ACP mode context
  • Consistent patterns: The fix follows existing patterns for path resolution (handling ~, relative paths, absolute paths)

🎯 Specific Feedback

🔵 Low

  • File: packages/core/src/utils/openaiLogger.ts:25-27 - The JSDoc mentions "In ACP mode, process.cwd() may be '/'" which is helpful context. Consider adding a link to issue ACP mode: Failed to initialize OpenAI logger #2671 in the comment for future reference:

     * @param cwd Optional working directory for resolving relative paths. Defaults to process.cwd().
     *            In ACP mode, process.cwd() may be '/' (filesystem root), so callers should
     *            pass the project working directory from Config.getWorkingDir().
     * @see https://github.com/QwenLM/qwen-code/issues/2671
     */
  • File: packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts:65-68 - Consider extracting the OpenAILogger instantiation to a private method for better testability and consistency with other initialization patterns in the class:

    private createOpenAILogger(
      generatorConfig: ContentGeneratorConfig,
    ): OpenAILogger | undefined {
      if (!generatorConfig.enableOpenAILogging) {
        return undefined;
      }
      return new OpenAILogger(
        generatorConfig.openAILoggingDir,
        this.config.getWorkingDir(),
      );
    }

✅ Highlights

  • Excellent test coverage: The new test cases in openaiLogger.test.ts thoroughly cover:

    • Default log directory with custom cwd
    • Relative path resolution against provided cwd
    • Absolute path behavior (ignores cwd)
    • Tilde expansion behavior (ignores cwd)
    • Fallback to process.cwd() when cwd is undefined
  • Minimal invasive change: Only 2 production files modified with surgical precision to fix the specific issue

  • Good test update: The loggingContentGenerator.test.ts mock update (getWorkingDir) ensures tests remain valid with the new interface

  • Clear PR description: The PR body provides excellent context with root cause analysis, fix summary, and testing instructions

@Mingholy Mingholy merged commit 4f8c985 into QwenLM:main Mar 27, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ACP mode: Failed to initialize OpenAI logger

2 participants