Skip to content

fix(service): tolerate locale dates in Windows service wrappers - #6298

Closed
hulkbig wants to merge 2 commits into
lidge-jun:devfrom
hulkbig:fix/windows-wrapper-locale
Closed

hulkbig wants to merge 2 commits into
lidge-jun:devfrom
hulkbig:fix/windows-wrapper-locale

Conversation

@hulkbig

@hulkbig hulkbig commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #6290. Also addresses the locale-date failure described in #5093.

  • Keep timestamp expansion outside parenthesized Windows batch blocks. Korean/Japanese dates containing parentheses currently abort the generated service wrapper before Bun starts, even when the missing-file guard would not execute.
  • Branch to top-level missing-runtime and restored-backup log labels. Preserve exit code 3 for missing runtime files, the stay-out/restart contract, transactional backup recovery, and disabled delayed expansion so exclamation marks in paths remain intact.
  • Add 24 real cmd.exe regression cases covering ordinary, Korean and Japanese date values across healthy startup, missing Bun/CLI, successful recovery, unusable backup, and standalone shapes. Update the lifecycle repair guidance and wrapper contract notes.

The test harness retains the generated loop, prelaunch guards and recovery code, supplies disposable paths, and uses a fake batch child with CALL so it returns like the real executable. It does not install or control an actual service. This proves the tested control flow; it is not an exhaustive Windows path-compatibility claim.

Verification

Based on current dev 6538001e2fdf54ef657f5057e8c3cbd3249d8e59. Current head: cb230f19232d464e8f35812d112e9388d4aa029a. Remote blob hashes for all four changed files match the locally checked candidate. Independent source review found no blocking issue.

Actual Windows before/after evidence, using the repository's unchanged manual ci.yml lane all on the contributor fork, Bun 1.4.0:

  • Before, test-only head 7d8a6745d5f6e3abc6308c9ded9adc558461ae82: all 8 ordinary-date controls passed; all 16 parenthesized-date cases failed with exit 255 instead of expected 0/3. Windows shard 7, batch 37/37
  • After, current head: all 24 new cases and the existing real-cmd exit-contract test passed. The complete Windows shard 7 succeeded. Fixed Windows shard 7
  • Full fixed run is complete and failed; this is not a full-matrix green claim. Eight of nine Windows shards passed, including the complete target shard 7. The only failed producer was Windows shard 2: provider outbound GET transport > proxy mode reaches one real proxy across outbound, connection-test, and model-discovery paths hit its 15-second timeout. Its fixture uses local loopback HTTP servers and synthetic destinations; the identical case passed in the baseline run in 1.9 seconds. This is an unrelated intermittent fixture timeout, not an established regression from this change. The baseline-only Cursor-auth failure passed in the fixed run. All other requested producer jobs, including both macOS shards, macOS control and desktop builds, passed. Aggregate ci correctly remains failed. No jobs were retried. Fixed full run

Local Linux checks:

  • bun run scripts/test.ts tests/windows/windows-service-wrappers.test.ts tests/service/standalone-service.test.ts tests/service/service-multishell-path.test.ts tests/service/service.test.ts: 234 passed, 25 Windows-only skipped, 0 failed
  • bun test tests/ci-workflows/file-size-ratchet.test.ts tests/test-layout.test.ts: 11 passed
  • bun run typecheck, bun run structure:check, bun run privacy:scan, git diff --check: passed
  • cd docs-site && ASTRO_TELEMETRY_DISABLED=1 bun run build: passed, 545 pages and 74,725 internal links checked
  • Full local suite was not duplicated because of concurrent checkout/resource use; focused checks ran locally and the existing complete hosted matrix was dispatched. Linux-skipped cases are not counted as Windows evidence.

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. Token handling and workflow permissions are unchanged; fixtures are disposable.

Review readiness checklist

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

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 30, 2026
@github-actions

github-actions Bot commented Sep 30, 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

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 63 / 80

이 PR은 한국어·일본어 Windows에서 서비스 래퍼가 Bun을 켜기 전에 죽어 버리는 버그를 고칩니다. 원인은 간단합니다. cmd.exe가 괄호 블록을 읽을 때 %DATE%를 먼저 펼치는데, 날짜에 )가 들어 있으면 블록이 중간에 닫혀 스크립트 전체가 깨집니다. 그래서 파일이 있어도 서비스가 안 뜹니다. 고친 방법은 로그에 시각을 찍는 줄을 괄호 블록 밖으로 옮기고, 없는 Bun/CLI·백업 복구 성공은 goto 라벨로 처리한 것입니다. 종료 코드 3, stay-out/재시작, delayed expansion 끔은 그대로 둡니다. 테스트는 일반·한국어·일본어 날짜로 건강한 시작, Bun/CLI 없음, 복구 성공·실패, standalone까지 24가지를 실제 cmd.exe로 돌리고, lifecycle·ops 문서에 업그레이드 후 ocx service repair 안내를 넣었습니다. base는 dev라서 맞습니다.

라인 - src/service/windows-taskxml.ts의 고침 방향은 맞고, 남은 %DATE% 로그 줄도 괄호 블록 밖이라 같은 함정은 없어 보입니다.
라인 - PR이 아직 draft이고 리뷰 준비 체크리스트가 전부 비어 있습니다. 머지 전에 작성자가 체크를 채워야 합니다.
라인 - 작성자도 적었듯 전체 Windows CI가 초록이라고 주장하지는 않습니다. 고쳐진 shard 7 증거는 있으나, 다른 shard 실패가 이 변경과 무관한지 메인테이너가 한 번만 보면 됩니다.
라인 - tests/windows/windows-service-wrappers.test.ts의 새 24케이스는 win32에서만 돌고 Linux CI에서는 전부 skip입니다. Windows 증거가 핵심이라 괜찮지만, 회귀는 Windows lane에 계속 묶여 있어야 합니다.
라인 - 이미 깔린 옛 래퍼는 코드만으로는 안 바뀝니다. 문서의 ocx service repair 경로가 실제 업그레이드·릴리스에서 사용자에게 충분히 닿는지가 남은 운영 포인트입니다.

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

draft 체크리스트와 남은 CI를 이 상태로 받아들일지, 아니면 ready 표시와 shard 정리를 더 기다릴지. 또 이미 서비스가 죽어 있는 ko-KR/ja-JP 사용자에게 repair 안내를 릴리스 노트에 얼마나 강하게 넣을지.

너의 추천

방향은 머지해도 될 만큼 분명합니다. 원인·고침·Windows 재현 테스트가 한 줄로 맞춰 있고 #6290/#5093와 직접 연결됩니다. 다만 draft·체크리스트·CI 정리는 작성자 쪽에서 끝낸 뒤 ready로 올린 다음에 머지하는 쪽이 안전합니다. types/config 분할이나 preview deploy 이야기는 이 PR과 무관합니다. 무효·중복으로 닫을 이유는 없습니다.

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

@lidge-jun

Copy link
Copy Markdown
Owner

Thanks @hulkbig. The root cause and the fix are right, and the real cmd.exe cases make this easy to trust. To get it into the next release, I carried both commits unchanged onto current dev as #6391. You stay the commit author, and the squash will carry a Co-authored-by trailer. That PR runs the full Windows shard lane at its own head. Once #6391 lands, a maintainer will close this one in its favor. Nothing more is needed from you.

lidge-jun added a commit that referenced this pull request Oct 1, 2026
…#6298) (#6391)

fix(service): tolerate locale dates in Windows service wrappers (carry #6298) (#6391)

On locales whose short date contains parentheses (ko-KR "2026-09-30(수)", ja-JP
"2026/09/19 (土)"), cmd.exe expands %DATE% while parsing a parenthesized block, so
the ")" closed the block early and the generated service wrapper aborted before it
launched Bun, on every start. The timestamped log lines now sit at top-level labels
(:bun_missing, :cli_missing, :backup_restored), outside any block. Exit code 3, the
stay-out/restart contract, transactional-backup recovery and disabled delayed
expansion are unchanged. Run "ocx service repair" after upgrading to regenerate an
installed wrapper. Closes #6290; same defect as #5093.

Carried from #6298.

Co-authored-by: Hulk <happyhls@gmail.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev through the maintainer carry #6391 (8a3a776), which keeps your authorship with a Co-authored-by trailer. Thank you!

@lidge-jun lidge-jun closed this Oct 1, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants