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

[lidge-jun]

Summary

Carries #6298 by @hulkbig onto current dev. Closes #6290 and fixes the same defect reported in #5093.

On Windows locales whose short date contains parentheses, such as ko-KR 2026-09-30(수) and ja-JP 2026/09/19 (土), the generated service wrapper opencodex-service.cmd aborts before it launches Bun. cmd.exe expands %DATE% while it parses a parenthesized block, before it evaluates the if condition. The ) in the date closes the block early, the rest becomes a syntax error, and the whole batch exits. This happens on every start, even when the guarded file exists. The scheduled task reports Last Run Result: 0, so the proxy silently never runs.

• The installation is incomplete and restored previous install log lines move out of parenthesized blocks into top-level :bun_missing, :cli_missing and :backup_restored labels. Exit code 3, the stay-out/restart contract, transactional-backup recovery and disabled delayed expansion are unchanged. Delayed expansion stays off, so ! in paths still works.
• 24 real cmd.exe regression cases cover ordinary, Korean and Japanese date values across healthy startup, missing Bun or CLI, successful restore, an unusable backup, and the standalone shape. They run only on win32.
• docs-site lifecycle reference and structure/ops/docs-and-release.md describe the rule and tell users to run ocx service repair after upgrading, because an already-installed wrapper is only regenerated by repair.

The two contributor commits are replayed unchanged with their author.

Carried from #6298. Original author: @hulkbig.

Co-authored-by: Hulk happyhls@gmail.com

Verification

• I did not run local test suites or typecheck. That is an explicit instruction from the project owner, not the AGENTS.md resource exception. Hosted CI at the exact head is the only execution evidence, including a manually dispatched lane=all run for the nine Windows shards that carry the new cases. The run will be linked here before merge.
• Contributor evidence on the fork (Bun 1.4.0): before the fix, all 16 parenthesized-date cases exited 255 (run); after the fix, all 24 cases passed in Windows shard 7 (run).
• The carry is git cherry-pick of 7d8a6745d5..cb230f1923 onto dev de8afe2e86, with no conflicts. git diff --check, bun run structure:check and bun run scripts/privacy-scan.ts pass. Neither changed source or test file is in the file-size baseline.

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. The wrapper still reads the token file the same way, and test fixtures use disposable temp paths.

Summary by CodeRabbit

• Bug Fixes
  • Fixed Windows service startup and backup recovery failures when locale-specific dates contain parentheses, including Korean and Japanese date formats.
  • Service repair now replaces older wrappers that could exit before launching Bun.
• Documentation
  • Clarified supported date formats and Windows service recovery behavior.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-01T12:56:23.662668Z	ab11937	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 3452bbb8-9c34-4f3e-aa79-e17e559536aa

📥 Commits

Reviewing files that changed from the base of the PR and between ab11937 and 4a2a475.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: bf8cb2da-3698-40a7-8f51-fb9217d9fa5e

📥 Commits

Reviewing files that changed from the base of the PR and between 7b2deb8 and ab11937.

📒 Files selected for processing (4)

• docs-site/src/content/docs/reference/cli/lifecycle.md
• src/service/windows-taskxml.ts
• structure/ops/docs-and-release.md
• tests/windows/windows-service-wrappers.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The Windows service wrapper now uses label-based checks for missing Bun and CLI executables and records the selected backup name before logging recovery. Windows tests cover locale dates and installation scenarios. Documentation describes the date-handling changes.

Changes

Windows service wrapper

Layer / File(s)	Summary
Wrapper checks and backup recovery src/service/windows-taskxml.ts	Missing-executable checks jump to shared labels that log the incomplete-install message and exit with code 3. Backup recovery stores the restored directory name and logs it at a shared success label.
Locale coverage and documentation tests/windows/windows-service-wrappers.test.ts, docs-site/src/content/docs/reference/cli/lifecycle.md, structure/ops/docs-and-release.md	Windows-only tests cover three locale date formats and eight installation scenarios. The documentation describes locale dates containing parentheses and the wrapper’s timestamp and delayed-expansion handling.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to ab119

The change makes the Windows service wrapper tolerate locale dates that contain parentheses, such as ko-KR and ja-JP, so it launches Bun as intended. No merge-blocking risk is visible. Existing installs only pick up the fix after running ocx service repair, and the docs already say so.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ab119

The locale fix preserves the service’s execution identity and normal recovery behavior. However, the new recovery log can interpret shell metacharacters in a backup directory name. Exploitation requires local backup-directory manipulation and an incomplete installation; no privilege escalation is established.

Retained concerns

• Low · security · inferred: The shared recovery log expands a filesystem-derived backup name as unquoted command text. During incomplete-install recovery, an actor able to create or rename a selected backup directory could place shell metacharacters in its name and cause additional commands to run under the service account. The prior FOR-variable log did not introduce this percent-expansion parsing step. Actual attacker reachability depends on filesystem permissions; no elevation beyond the existing service identity is established.

Security review details

Security Blast Radius

• inferred — Any injected command would execute within the existing least-privilege service account and inherit the wrapper’s environment, which may include its loaded API authentication token. Exposure is therefore bounded by that account’s accessible resources and credentials, not an established administrator or cross-user privilege transition. Actual filesystem permissions remain unverified.

Security Findings and Attack Paths

• inferred — The conditional attack path is local backup-name manipulation, followed by missing-executable recovery and successful package.json validation, then percent expansion of the selected name into the top-level echo command. A name containing an ampersand can introduce command syntax. This parsing exposure is new; runtime exploitation and access by a lower-trust actor have not been demonstrated.

Trust Boundaries and Controls

• observed — Repair rejects unknown or foreign service ownership and checks the service environment and authentication before rewriting assets. Those publication controls do not validate names consumed later by recovery. Legitimate timestamp-only names and the existing trust in restored package executables are important counterevidence against claiming a newly established high-privilege boundary crossing.

Resilience and Maintainability Implications

• inferred — The label transition preserves the called recovery subroutine’s return behavior and leaves delete/move ordering, executable checks, and terminal exits unchanged. Non-atomic filesystem mutation, suppressed operation errors, and package.json-based success detection predate this PR. Their absolute interruption and concurrency safety remains unresolved, but the comparison does not establish a new regression in those operations.

Hardening Proposals

• proposed — Preserve locale-safe timestamp logging while preventing backup names from becoming command syntax: validate selected names against the producer’s fixed format or use a logging mechanism that does not reparse filesystem text. Add an adversarial backup-name case to distinguish literal logging from command execution.

🚥 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 1 functions across 2 files. (2 skipped: 2 …	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 clearly identifies the main change: fixing Windows service wrappers so locale-specific dates with parentheses do not abort service startup or recovery.
Linked Issues check	✅ Passed	The PR meets the coding objective in [#6290] and addresses the same ja-JP defect referenced from [#5093]. In src/service/windows-taskxml.ts:104-128, the missing Bun and CLI paths now use goto labe…
Out of Scope Changes check	✅ Passed	The changed files remain within the linked issue scope. The wrapper changes implement the Windows service lifecycle fix. The Windows integration tests validate the affected parsing, startup, missing-f…

Full details: Docstring Coverage

Explanation

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 1 functions across 2 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[lidge-jun]

Maintainer integration record (MAINTAINERS.md, dev-only)

• Decision: integrate fix(service): tolerate locale dates in Windows service wrappers (carry #6298) #6391, the maintainer carry of fix(service): tolerate locale dates in Windows service wrappers #6298 by @hulkbig that closes [Bug][Windows] Service wrapper still aborts before launching bun when %DATE% contains parentheses (ko-KR) — #5093 unfixed in 2.73.0 #6290 (the same defect as [Bug][Windows] Service wrapper fails to parse on locales where %DATE% contains parentheses (e.g. ja-JP); proxy never starts under Task Scheduler #5093), into dev without a second maintainer approval, as the project owner authorized on 2026-10-01.

• Exact head: 4a2a475d0226b952e63d388e7c65a86327e1af81, rebased onto dev 404ca8b551. It is 8 commits behind the current dev, and git merge-tree --write-tree is clean.

• Windows evidence at this head: in dispatched lane=all run 36881532418, job 110434332502 (a Windows shard that succeeded) passed all 24 cmd prelaunch with <date> cases. Those cover ordinary, ko-KR 2026-09-30(수) and ja-JP 2026/09/19 (水) dates across healthy, missing Bun or CLI, restore, empty-backup and standalone shapes, plus the existing real-cmd exit-contract test. The earlier head ab11937562 showed the same in run 36864870380. windows 3/9 in run 36881532418 failed only in sibling-home-client-sync on a slow runner, which is the flake test(config): canonicalize the owner-registry ACL test root (macOS) #6411 documents and which passed at the same base in two other runs.

• PR-lane run 36881540243: every job passed except macos 2/2 and the ci aggregate. macos 2/2 failed only in owner-registry-acl.test.ts:72, the dev defect from fix(config): require owner-only no-follow registry publication #6375 that test(config): canonicalize the owner-registry ACL test root (macOS) #6411 fixes. Passing jobs include test 1/4 to test 4/4, gates, structure gate, macos 1/2, docs site build, the keyring and npm-global jobs on all three OSes, docker smoke, desktop shell and macos widget + bundle. React Doctor run 36881540140 succeeded. For a fully green aggregate, land test(config): canonicalize the owner-registry ACL test root (macOS) #6411 first, then rebase or re-run.

• Local suites: not run, by explicit owner instruction.

• Security review: not required. The wrapper reads the token file exactly as before, and the change only moves timestamp log lines out of parenthesized blocks.

• Attribution: two commits replayed unchanged, authored by Hulk happyhls@gmail.com. The squash must carry the trailer below. After merge, close fix(service): tolerate locale dates in Windows service wrappers #6298 in favor of fix(service): tolerate locale dates in Windows service wrappers (carry #6298) #6391, and close [Bug][Windows] Service wrapper still aborts before launching bun when %DATE% contains parentheses (ko-KR) — #5093 unfixed in 2.73.0 #6290 manually. Release notes should tell ko-KR and ja-JP users to run ocx service repair, because an already-installed wrapper is only regenerated by repair.

• Accepted non-green jobs: macos 2/2|ci|macos control. Their only failure is tests/config/owner-registry-acl.test.ts, which test(config): canonicalize the owner-registry ACL test root (macOS) #6411 (merged just before this) fixes. The lane=all Windows run 36881532418 passed all 24 locale cases and the exit-contract test at this head. The post-merge dev Cross-platform CI covers the union.