Skip to content

fix(claude): avoid creating agents directory when disabled - #2619

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
fflake33:codex/fix-disabled-claude-dir
Aug 25, 2026
Merged

lidge-jun merged 1 commit into
lidge-jun:devfrom
fflake33:codex/fix-disabled-claude-dir

Conversation

@fflake33

@fflake33 fflake33 commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Avoid creating ~/.claude/agents when the Claude integration has no definitions to write and the directory does not already exist.
  • Preserve cleanup of marker-owned stale definitions when the directory already exists, and preserve normal writes for non-empty rosters.
  • Add a focused filesystem regression test for the absent-directory contract. This is a follow-up to [Feature] synchronize Claude Code agent definitions during proxy startup #2200.
  • No documentation change is needed because this removes an unintended filesystem side effect without changing the configured integration surface.

Verification

  • bun test tests/claude-agents-inject.test.ts — 18 passed.
  • bun test tests/claude-agent-startup-sync.test.ts — 5 passed.
  • bun run typecheck — passed.
  • Isolated-home startup replay with claudeCode.enabled=false — returned no definitions and left .claude absent.
  • bun run test — the 4x parallel phase reported 14,814 passed, 12 skipped, 15 failed, and 3 errors; every listed retry file passed in the subsequent 1x serial reruns, but the command exited 1. The reported failures are outside this PR's two-file diff.
  • bun run privacy:scan — blocked by pre-existing email examples in docs-site/src/content/docs/reference/cli/providers-accounts.md:167-168, which this PR does not modify.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes

    • Prevented creation of an empty agents directory when no agent definitions are available.
    • Preserved cleanup of existing directories and continued reporting unexpected filesystem errors.
  • Tests

    • Added regression coverage confirming empty synchronization results in no directory being created.

An empty roster has nothing to write, and an absent agents directory has nothing to prune. Return without creating ~/.claude while preserving cleanup for existing OpenCodex-owned definitions.
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 25, 2026
@coderabbitai

coderabbitai Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 458fed54-f668-47ee-9aca-f8e4f7cda732

📥 Commits

Reviewing files that changed from the base of the PR and between eca432d and 137c389.

📒 Files selected for processing (2)
  • src/claude/agents-inject.ts
  • tests/claude-agents-inject.test.ts

📝 Walkthrough

Walkthrough

syncClaudeAgentDefs now returns an empty list without creating the agents directory when no definitions exist and the directory is absent. Existing-directory cleanup and non-ENOENT error handling remain unchanged. A regression test verifies this behavior.

Changes

Claude agent synchronization

Layer / File(s) Summary
Handle empty definitions without directory creation
src/claude/agents-inject.ts, tests/claude-agents-inject.test.ts
At lines 221–228, empty definitions avoid creating an absent agents directory. The test imports existsSync and verifies that syncClaudeAgentDefs([], dir) returns [] while leaving the directory absent.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: lidge-j

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked.

@lidge-jun
lidge-jun marked this pull request as ready for review August 25, 2026 20:43
@lidge-jun
lidge-jun merged commit 1a92d6b into lidge-jun:dev Aug 25, 2026
5 of 6 checks passed
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 64 / 80

설명: 이 풀은 클로드 통합이 꺼져 있거나 쓸 정의가 없을 때 ~/.claude/agents 폴더를 새로 만들지 않게 한다. 작성자는 fflake33 이다. 지금 열려 있고 드래프트다. 라벨은 bug 다. 베이스는 dev 이고 헤드는 codex/fix-disabled-claude-dir 다. 지금 CURRENT dev HEAD 는 1a15d62 이다. GitHub mergeable 은 true 다. mergeState 는 BLOCKED 다. 드래프트라서다. 본문은 2200 의 이어서라고 적는다. 파일이 두 개다. 더하기 15, 빼기 2. 체크리스트 네 칸이 모두 비어 있다. 내가 구현하지 않고 머지하지 않는다. 태그하지 말 것. 배포하지 말 것.

HEAD 에서 구멍은 그대로다. src/claude/agents-inject.ts 218줄 syncClaudeAgentDefs 는 221줄에서 정의가 없어도 mkdirSync 를 recursive 로 부른다. 249줄 injectClaudeAgentDefs 는 claudeCode.enabled 가 false 이거나 injectAgents 가 false 이면 253줄에서 빈 배열로 sync 를 부른다. 그래서 통합을 꺼도 agents 폴더가 생긴다. src/cli/claude-agent-startup-sync.ts 와 src/server/system-env.ts 416줄, src/cli/claude.ts 330줄이 그 길을 탄다. tests/claude-agents-inject.test.ts 322줄은 꺼진 뒤에 폴더 안 파일이 비었는지만 본다. 폴더 자체가 없었는지는 안 본다.

이 풀은 빈 정의일 때만 먼저 lstatSync 로 폴더를 본다. 없으면 ENOENT 에서 빈 배열을 바로 돌린다. 폴더가 이미 있으면 예전처럼 읽어 소유 파일을 지운다. 정의가 있으면 mkdirSync 를 그대로 둔다. tests/claude-agents-inject.test.ts 에 없는 폴더를 안 만든다는 시험을 더한다. 범위가 작고 2200 이어서와 맞다. package.json 을 안 올린다. 위생 차단 대상이 아니다. 큰 포크 덤프도 아니다. types.ts/config.ts 가르기를 깨지 않는다.

다만 드래프트다. 본문 체크리스트 네 칸이 비어 있다. 자동 댓글도 칸을 다 채울 때까지 드래프트로 두라고 한다. 본문이 적은 bun run test 는 다른 파일에서 실패가 났고 재시도는 통과했다고 한다. 이 풀의 두 파일 밖이다. 그 칸을 작성자가 채우기 전에는 합치지 말 것. 이번 시간 착지 2610 부터 2621 은 agents-inject.ts 를 안 건드린다. 리베이스가 필수는 아니다. 2200 을 이 풀로 닫지 말 것. 내가 확인하지 않은 이슈를 닫지 않는다. src/runtime 은 없다. default-aliases.ts 는 이제 있다. 이 풀의 주제가 아니다.

라인 221 - src/claude/agents-inject.ts HEAD 는 정의가 없어도 mkdirSync 를 부른다
라인 253 - injectClaudeAgentDefs 가 꺼진 통합에서 빈 배열 sync 를 탄다. 그래서 폴더가 생긴다
라인 218 - 이 풀은 빈 정의일 때 없는 폴더를 만들지 않는다. 있는 폴더는 지우는 길을 남긴다
라인 249 - enabled false 와 injectAgents false 둘 다 같은 빈 sync 다. 이 풀이 그 계약을 지킨다
tests/claude-agents-inject.test.ts - 없는 agents 폴더가 없는 채로 남는 시험을 더한다
라인 416 - src/server/system-env.ts 프록시 시작이 inject 를 탄다. 이 구멍이 시작 때 터진다
2200 - 이어서다. 이 풀로 2200 을 닫지 말 것
체크리스트 - 네 칸이 비어 있다. 드래프트를 유지한다

메인테이너의 판단이 필요한 지점

  • 이 풀을 받을지. 구멍과 고침이 맞다. 다만 드래프트다
  • 체크리스트를 기다릴지. 기다리는 편이 맞다
  • 2200 을 같이 닫을지. 닫지 말 것
  • HEAD 위로 리베이스할지. 파일 겹침은 없다. 필수는 아니다
  • 위생 차단을 붙일지. 붙이지 말 것. 범위가 작다
  • 지금 leftover-close 할지. 차량이 아니다. 내가 닫지 않는다

너의 추천
구멍은 받는다. 빈 정의에서 없는 폴더를 만들지 않는 위치가 맞다. 있는 폴더의 소유 파일 지우기는 남긴다. 작성자가 체크리스트를 채우고 드래프트를 푼 뒤에 메인테이너가 bun test tests/claude-agents-inject.test.ts 만 확인하면 된다. 지금 합치지 말 것. 내가 머지하지 않는다. 라벨은 그대로 둔다. 프리뷰 배포가 아니다.

이 댓글은 grok-bot이 작성했습니다

@fflake33
fflake33 deleted the codex/fix-disabled-claude-dir branch August 27, 2026 17:19
tarunravi pushed a commit to tarunravi/opencodex that referenced this pull request Sep 14, 2026
…2619)

An empty roster has nothing to write, and an absent agents directory has nothing to prune. Return without creating ~/.claude while preserving cleanup for existing OpenCodex-owned definitions.
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
…2619)

An empty roster has nothing to write, and an absent agents directory has nothing to prune. Return without creating ~/.claude while preserving cleanup for existing OpenCodex-owned definitions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants