Skip to content

fix: break 3-file import cycle in cli/ - #904

Open
AbhijitK20 wants to merge 1 commit into
CodebuffAI:mainfrom
AbhijitK20:fix-import-cycles
Open

fix: break 3-file import cycle in cli/#904
AbhijitK20 wants to merge 1 commit into
CodebuffAI:mainfrom
AbhijitK20:fix-import-cycles

Conversation

@AbhijitK20

Copy link
Copy Markdown

project-files.ts was importing getConfigDir from ./utils/auth, which imported logger, which imported back to project-files. Changed the import to ./utils/config-dir which has no transitive dependencies on project-files, breaking the cycle:

project-files.ts -> auth.ts -> logger.ts -> project-files.ts

Now: project-files.ts -> config-dir.ts (no cycle)

project-files.ts was importing getConfigDir from ./utils/auth, which
imported logger, which imported back to project-files. Changed the
import to ./utils/config-dir which has no transitive dependencies on
project-files, breaking the cycle:

  project-files.ts -> auth.ts -> logger.ts -> project-files.ts

Now: project-files.ts -> config-dir.ts (no cycle)
@codebuff-team

Copy link
Copy Markdown
Contributor

Thanks for tracking down the cycle — this is exactly the kind of minimal, mechanical fix that's easy to verify and low-risk to port. The change itself is a one-line import swap, which is good.

A couple of things a maintainer will want to confirm before porting:

  1. That ./utils/config-dir actually exports getConfigDir with the same signature/behavior as the one currently re-exported (or duplicated) from ./utils/auth. If auth.ts re-exports getConfigDir from config-dir.ts rather than defining it, this is a safe no-op swap. If auth.ts wraps or shadows it with different logic, this could silently change behavior.
  2. Whether the cycle actually causes a real problem today (e.g. runtime error from circular require, or just a bundler/tree-shaking wart). The PR description asserts a cycle but doesn't show a failing test or reproduction — worth adding a one-line note on what symptom this fixes (crash, wrong logger init order, etc.) so the maintainer doesn't have to re-derive it.
  3. No test coverage — understandable for an import-order fix, but if there's an existing test that exercises project-files.ts at import time, it'd be good to confirm it still passes.

Given the tiny diff and clear rationale, this looks like a reasonable candidate to port after a maintainer double-checks that config-dir.ts's getConfigDir is identical to the one previously imported via auth.ts.

@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 20, 2026
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.

2 participants