Skip to content

fix: prevent Windows config writes from failing with ENOENT - #3398

Closed
x3M3x wants to merge 1 commit into
lidge-jun:devfrom
x3M3x:codex/fix-config-write-race
Closed

fix: prevent Windows config writes from failing with ENOENT#3398
x3M3x wants to merge 1 commit into
lidge-jun:devfrom
x3M3x:codex/fix-config-write-race

Conversation

@x3M3x

@x3M3x x3M3x commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Use Node's portable exclusive-write flag for OpenCodex atomic temp files instead of numeric open flags that Bun on Windows can misreport as ENOENT even when the config directory exists.
  • Covers both synchronous startup/config writes and asynchronous state writes while preserving exclusive creation, 0600 permissions, and Windows ACL hardening.

Verification

  • �un test tests/config.test.ts (169 pass)
  • �un run typecheck
  • �un run test
  • �un run privacy:scan

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed (not needed; no user-facing configuration change).
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults; exclusive creation and existing file hardening remain intact.

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

  • Refactor
    • Standardized temporary-file creation across synchronous and asynchronous operations while preserving existing file permissions and behavior.

@github-actions github-actions Bot added bug Something isn't working intake: hygiene-blocked Deterministic PR hygiene checks failed labels Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • missing_regression_test — Behavior changed under src/ or gui/src/ without a test change. Add focused coverage or obtain test-exception-approved.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • hygiene: missing_regression_test.

What to do

  • Fix missing_regression_test — Behavior changed under src/ or gui/src/ without a test change. Add focused coverage or obtain test-exception-approved.
  • 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.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.
@x3M3x Tick the boxes once your local CI is green, your branch is on the latest dev commit, and every correct Codex and CodeRabbit finding is resolved.

@github-actions
github-actions Bot marked this pull request as draft September 3, 2026 19:19
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: a840fd94-b817-4635-b598-af2390e30cc4

📥 Commits

Reviewing files that changed from the base of the PR and between 59bc82f and e979550.

📒 Files selected for processing (1)
  • src/config/atomic-write.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change replaces numeric fs flags with the equivalent "wx" mode in both private temporary-file writers. The 0o600 permission mode remains unchanged.

Changes

Atomic write behavior

Layer / File(s) Summary
Update temporary-file open modes
src/config/atomic-write.ts:123, src/config/atomic-write.ts:144
writePrivateTempFile and writePrivateTempFileAsync now call openSync with "wx" and retain 0o600 permissions. The resulting write-only, create-exclusive behavior remains unchanged.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to e9795

Configuration and state writes now use a portable exclusive-write mode on Windows while retaining private-file permissions and atomic-write behavior. No merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the main change: replacing non-portable temporary-file open flags to prevent Windows configuration writes from failing with ENOENT. It is concise and specific, although…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title accurately identifies the main change: replacing non-portable temporary-file open flags to prevent Windows configuration writes from failing with ENOENT. It is concise and specific, although it does not mention the related asynchronous state-write path.

  • Fix all pre-merge checks with AI
✨ 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.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 61 / 80

이 PR은 Windows에서 설정 파일을 저장할 때, 폴더가 있는데도 ENOENT(파일이 없다)로 실패하는 길을 막습니다. 지금 dev HEAD는 59bc82f45(#3396 Startup, 패키지 2.42.0)입니다. 설정 저장은 src/config/atomic-write.ts가 임시 파일을 만들고 이름을 바꿉니다. 임시 파일 이름은 대상 옆에 .ocx.<pid>.<번호>.tmp 입니다. 그 파일을 열 때 지금은 Node의 숫자 플래그 O_WRONLY | O_CREAT | O_EXCL 을 씁니다. 뜻은 쓰기 전용, 없으면 만들기, 이미 있으면 거절입니다. 권한은 0o600입니다. Windows에서는 만든 뒤 hardenSecretPath로 ACL을 붙입니다. POSIX에서는 fchmodSync로 주인만 읽게 합니다. 그 다음에 내용을 쓰고, 대상 이름으로 바꿉니다. 숫자 플래그를 Bun이 Windows에서 잘못 읽으면, 폴더가 있어도 열기가 ENOENT로 떨어집니다. 그러면 시작 때 설정 저장과 비동기 상태 저장이 같이 깨집니다. 대시보드 미니멀 열차(#3391·#3393·#3395·#3396)와는 파일이 겹치지 않습니다. src/types.ts / src/config.ts 분할과도 무관합니다. close-don't-rebase 대상이 아닙니다.

고치는 방법은 같은 뜻을 문자열 플래그 wx 로 적는 것입니다. Node와 Bun 문서에서 wx 는 쓰기 + 만들기 + 이미 있으면 실패입니다. 숫자 조합과 같은 뜻입니다. 파일이 없어야만 열리므로 잘라내기(truncate)는 실제로 일어나지 않습니다. 권한 인자 0o600 은 그대로입니다. Windows ACL 호출도 그대로입니다. 바뀌는 곳은 동기 writePrivateTempFile 과 비동기 writePrivateTempFileAsync 두 곳뿐입니다. constants import는 그 두 호출 말고는 안 쓰여서 빠집니다. 저장소 다른 곳은 이미 이 문자열을 씁니다. 예: src/responses/spill-store.ts 559·623행, src/server/management-auth.ts 159행, src/oauth/store.ts 잠금 생성, src/codex/account-store.ts 489행, src/config.tscreateExclusiveflag: "wx" 입니다. 이 PR은 설정 원자 저장만 그 관례에 맞춥니다.

이미 있는 테스트 tests/config.test.ts 의 Windows ACL 묶음은 비밀 바이트가 처음 보일 때 비공개인지, 미리 있는 임시 파일을 거절하는지를 봅니다. 2749행 테스트는 임시 파일이 이미 있으면 atomicWriteFile 이 던지고, 그 기존 파일 내용은 그대로인지를 확인합니다. 그 거절은 wx 로 바꿔도 같아야 합니다. 그래서 기존 테스트는 이 버그를 잠그지 못합니다. 숫자 플래그로 되돌려도 그 테스트는 통과할 수 있습니다. 게이트가 missing_regression_test 로 막은 이유가 그것입니다. openSyncwx0o600 으로 불리는지를 두 헬퍼 모두에서 잠그면 됩니다. 소스 문자열을 보거나, openSync 를 스파이하면 됩니다. Windows 실기에서 Bun ENOENT 를 재현하는 테스트는 이 CI에서 돌릴 수 없습니다. 플래그 문자열을 잠그는 쪽이 이 저장소 관례와 맞습니다.

이 PR은 초안입니다. 라벨 intake: hygiene-blocked 이 붙어 있습니다. 본문 체크리스트는 0/4입니다. GitHub merge 상태는 BLOCKED입니다. 충돌은 없습니다. 연결된 이슈 번호는 없습니다. 본문은 bun test tests/config.test.ts 169통과와 typecheck, test, privacy:scan을 적었습니다. 게이트가 테스트 파일 변경을 요구하므로, 지금 상태로는 리뷰 준비 라벨이 안 붙습니다. 초안을 유지한 채 테스트를 넣는 것이 맞습니다. 이 댓글에서 라벨은 바꾸지 않습니다.

범위 밖이지만 알아둘 숫자 플래그가 남아 있습니다. src/lab/ledger/store.tssrc/lab/artifacts/secure-fs.ts , src/lab/fabric/scratch.ts 는 아직 O_CREAT | O_EXCL 숫자 조합을 씁니다. 일부는 디렉터리 플래그와 같이 가서 문자열 wx 로 못 바꿉니다. 설정 원자 저장과 같은 Windows 경로가 아닙니다. 이번 2줄 수정에 묶지 않는 편이 맞습니다. src/codex/native-main-lock-file.tsO_RDWR | O_CREAT 이고 배타 생성이 아닙니다. 이번 버그와 별개입니다.

src/config/atomic-write.ts writePrivateTempFile 약 120행 - 플래그를 wx 로 바꿉니다. 테스트가 이 문자열을 잠그지 않으면 숫자 플래그로 되돌려도 기존 ACL 테스트는 통과합니다.

src/config/atomic-write.ts writePrivateTempFileAsync 약 141행 - 동기 헬퍼와 같은 변경입니다. 비동기 상태 저장도 같은 Windows ENOENT 를 탈 수 있는데, 한쪽만 잠그면 다른 쪽이 다시 깨질 수 있습니다.

tests/config.test.ts 2749행 - 미리 있는 임시 파일 거절만 봅니다. Bun Windows가 숫자 플래그를 ENOENT 로 잘못 보고하는 회귀는 잡지 못합니다.

PR 본문 체크리스트 0/4, 라벨 intake: hygiene-blocked - 초안이고 게이트가 테스트 부재로 막았습니다. 지금 머지하면 안 됩니다.

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

  • 회귀 테스트를 openSync 스파이로 둘지, 소스에 wx 문자열이 있는지로 둘지.
  • lab 쪽 남은 숫자 O_EXCL 을 별도 이슈로 쓸지, 이번엔 설정 경로만 고칠지.
  • 추적 이슈를 새로 열고 이 PR이 닫게 할지, 이슈 없이 버그 라벨만으로 갈지.

너의 추천
초안을 유지하세요. writePrivateTempFilewritePrivateTempFileAsync 둘 다 openSync(..., "wx", 0o600) 을 부르는 테스트를 넣으면 게이트가 풀립니다. 그다음 CI가 초록이면 dev에 머지해도 됩니다. 지금은 머지하지 마세요. types/config 분할로 닫을 대상이 아닙니다. 중복 PR도 없습니다. lab 숫자 플래그는 이 PR에 넣지 마세요.

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

@x3M3x

x3M3x commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #3383. That PR already contains the same portable wx Windows atomic-temp-file fix plus the focused regression test, so it is the one reviewable implementation to keep.

@x3M3x x3M3x closed this Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working intake: hygiene-blocked Deterministic PR hygiene checks failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants