Skip to content

fix(service): make Windows scheduler status locale independent - #736

Closed
luvs01 wants to merge 2 commits into
lidge-jun:devfrom
luvs01:fix/722-service-status-output
Closed

fix(service): make Windows scheduler status locale independent#736
luvs01 wants to merge 2 commits into
lidge-jun:devfrom
luvs01:fix/722-service-status-output

Conversation

@luvs01

@luvs01 luvs01 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace user-facing localized schtasks table output with a locale-independent Windows scheduler status
  • combine the existing tri-state task-registration probe with the identity-checked OpenCodex /healthz probe
  • report scheduler registration and proxy runtime as independent states, including a proxy running without a registered scheduler task, without exposing localized query errors
  • document the Windows status behavior in the canonical CLI reference and all maintained translations

Fixes #722

Verification

  • bun scripts/test.ts tests/service.test.ts tests/windows-scheduler-install-verification.test.ts (58 passed, Bun 1.3.14)
  • bun x tsc --noEmit
  • bun scripts/privacy-scan.ts
  • cd docs-site && bun install --frozen-lockfile && bun run build (146 pages)
  • git diff --check
  • The initial monolithic Windows run was cut off by a 10-minute local command budget. Current CI documents an approximately 11.8-minute Windows baseline and uses a 20-minute job budget (CI: windows-latest test job cancelled at the 12-minute ceiling, reported as a failing check #717); isolated current-dev slices continued making progress and completed, confirming a harness cutoff rather than a deadlock. The separate 5-second cli-restore-back test budget found during diagnosis is already covered by open security: harden post-merge service and management boundaries #707. Cross-platform CI still requires repository approval for this fork PR.

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.

Summary by CodeRabbit

  • New Features
    • Improved ocx service status on Windows by separately reporting Task Scheduler registration and verified OpenCodex proxy availability.
    • Windows status summaries no longer display localized schtasks tables, keeping output readable across system code pages.
  • Documentation
    • Updated CLI reference documentation in English, Japanese, Korean, Russian, and Simplified Chinese with the revised Windows behavior.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Windows ocx service status now uses structured scheduler and proxy probes, reports their states independently, avoids localized schtasks tables, and documents this behavior in the English, Japanese, Korean, Russian, and Simplified Chinese CLI references.

Changes

Windows service status

Layer / File(s) Summary
Structured scheduler and proxy inspection
src/service.ts, tests/windows-scheduler-install-verification.test.ts
Adds injectable task and proxy probes, tri-state proxy status formatting, locale-independent error handling, and regression coverage for localized or garbled scheduler output.
CLI integration and localized documentation
src/service.ts, docs-site/src/content/docs/*/reference/cli.md
Routes Windows scheduler status through the new inspector and documents separate task-registration and proxy-reachability reporting without localized schtasks tables.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: bug

Suggested reviewers: wibias, ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code and tests match #722 by replacing localized schtasks output with locale-independent status reporting.
Out of Scope Changes check ✅ Passed The changes stay within service logic, tests, and docs, with no unrelated scope visible in the summary.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: locale-independent Windows scheduler status handling in service code.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions github-actions Bot added the bug Something isn't working label Jul 30, 2026
@luvs01
luvs01 marked this pull request as ready for review July 30, 2026 06:50
@lidge-jun

Copy link
Copy Markdown
Owner

LAND-AFTER — approved in principle, queued behind the release blockers.

This resolves #722 correctly, and the approach is the right one. Rather than trying to decode CP949 out of schtasks, src/service.ts:409-437 stops depending on the localized table at all: registration state and proxy liveness are formatted from OpenCodex-owned strings, and :1823-1829 drops the raw table from user output entirely. tests/windows-scheduler-install-verification.test.ts:49-66 proves the point by feeding in the exact mojibake from the issue and asserting it never reaches the output — an exclusion assertion, which is harder to write and more convincing than a positive one.

Worth stating plainly since it is a contract change: status no longer shows the raw scheduler table. Users lose the per-field detail that table carried, and gain a summary that reads the same in every locale. That is the trade the issue asked for, and the docs update across locales is already in the PR.

What happens next: nothing is required from you. The two release blockers (#749, #737) are now on dev. This lands after them, once the branch is refreshed against current dev and the required checks re-run.


CI context (shared across the current review round). dev is now at 62e937614. The baseline breakage that made every required check red is fixed: run 30545575865 has macos-latest, ubuntu-latest, and all three npm-global jobs green. The one remaining red is windows-latest, and it is not a test failure — it is a Bun 1.3.14 runtime panic (heap.zig:deleteMin reached through spawnSync with a timeout, triggered by our Windows ACL hardening path). That is a runtime defect being tracked separately, so please do not treat a red windows-latest as a signal about your change. Rebase onto current dev before your next push so your checks run against the repaired baseline.

@lidge-jun

Copy link
Copy Markdown
Owner

Closing as superseded — the fix is already on dev, and merging this branch now would undo it.

1d9e196e7 fix(service): make Windows status locale independent landed the same outcome: ocx service status on the scheduler backend goes through inspectWindowsSchedulerServiceStatus() (src/service.ts:442-492, called at :1965-1972), which prints a structured summary built from a task probe plus an identity-checked proxy probe. No localized schtasks table reaches the user, so the mojibake in #722 is gone regardless of code page.

The reason this needs to be closed rather than merged: dev also carries decodeSchtasksOutput() at src/service.ts:364-393, which handles the UTF-16LE (often BOM-prefixed) output of schtasks /query /xml. git diff dev <this branch> -- src/service.ts shows that merging this head deletes that decoder and restores encoding: "utf8" in runFile(). That would break every scheduler health check and roll back successful elevated installs — reopening #722 by a different route.

Thanks for the fix; the direction was right and it shaped what landed. #722 is closed with the evidence above.

@lidge-jun lidge-jun closed this Jul 31, 2026
Wibias pushed a commit to Wibias/opencodex that referenced this pull request Jul 31, 2026
The dev checkout is mid-optimization, so this round stages on
codex/260731-pr-merge-round instead of dev. The user merges it into dev
and releases once that unit lands.

Four reviewers triaged every open PR against origin/dev=356924263 on
disjoint slices, and the load-bearing finding is that seven PRs are
already fixed on dev by different commits. Merging those heads now would
REVERT the newer work. lidge-jun#736 is the clearest case: dev carries
decodeSchtasksOutput() at src/service.ts:364-393 because schtasks
/query /xml emits UTF-16LE, and the PR head deletes that block and
restores encoding: "utf8" -- landing it reopens lidge-jun#722. A clean merge-tree
is not permission to merge.

An adversarial audit then returned five blockers, all folded in rather
than argued away. Two changed what actually gets merged. lidge-jun#744 was routed
as a routine catalog fix, but 59d95c0 and 39543a3 change OAuth
reconciliation, persist provider settings, and move token resolution
around the static branch -- security review per MAINTAINERS.md, so it
left the batch. lidge-jun#781's topic commit also swaps three /api/logs test files
onto a logsFromApiBody helper that accepts both the array and the {logs}
envelope; adopting it would pre-accept the very contract batch B rejects,
so only the Anthropic file and its own test come across.

The audit also caught three PRs missing from a matrix that claimed to be
complete (lidge-jun#750, lidge-jun#746, lidge-jun#644), and lidge-jun#644 additionally carries
.codexclaw/goalplans/** and two .DS_Store files.

Nothing is merged yet. Batch A is six PRs, batch B rebuilds three whose
implementations no longer fit the tree, and 20+ are held for security
review, provider evidence, or their own cycle -- each with what would
unblock it recorded.
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