Skip to content

fix(scripts): route i18n formatter through pnpm runner#95534

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
ooiuuii:fix/control-ui-i18n-pnpm-runner
Jun 27, 2026
Merged

fix(scripts): route i18n formatter through pnpm runner#95534
vincentkoc merged 1 commit into
openclaw:mainfrom
ooiuuii:fix/control-ui-i18n-pnpm-runner

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Route control-ui-i18n generated TypeScript formatting through the shared scripts/lib/format-generated-module.mjs helper.
  • Reuse the existing direct oxfmt plus pnpm-runner fallback path instead of spawning bare pnpm from control-ui-i18n.
  • Preserve the replacement-character restoration guard used by locale generation.

What Problem This Solves

On Windows, pnpm ui:i18n:check can fail during generated locale formatting with spawn pnpm ENOENT even when pnpm is available through the usual .cmd shim. The script fell back to spawning bare pnpm, bypassing the repository's existing scripts/pnpm-runner.mjs portability logic.

The fix keeps generated formatter command resolution in the existing shared formatter helper, avoiding a second ownership path for oxfmt and pnpm-runner behavior.

Linked context

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: control-ui-i18n formatting could reach a bare spawn("pnpm", ...) fallback on Windows, causing spawn pnpm ENOENT instead of using the repo's Windows-safe pnpm runner.
  • Real environment tested: Windows PowerShell checkout at F:\openclaw\worktrees\openclaw-control-ui-i18n-pnpm-runner, rebased onto current origin/main, hydrated with pnpm install --frozen-lockfile.
  • Exact steps or command run after this patch: pnpm ui:i18n:check
  • Evidence after fix: The command completed through all 18 locales and ended with clean locale output, including fa: clean (fallbacks=0), without spawn pnpm ENOENT.
  • Observed result after fix: Generated TypeScript formatting now goes through the shared generated-module formatter path, which already wraps Windows pnpm .cmd execution through the repo pnpm runner.
  • What was not tested: I did not run the full CI matrix locally. The focused Vitest run covers control-ui-i18n process behavior and the shared format-generated-module Windows pnpm-runner contract; the real command validates the end-to-end i18n check on Windows.
  • Proof limitations or environment constraints: The command used fallback-only translation mode because no provider credentials were configured; the bug and fix are in local formatter process spawning, not translation provider calls.
  • Before evidence: Before this patch, local Windows validation reached the formatter step and failed with spawn pnpm ENOENT.

Evidence

After this patch, pnpm ui:i18n:check completes on Windows:

control-ui-i18n: command=check locales=18 provider=fallback-only model=n/a
...
fa: clean (fallbacks=0)

The focused test run also includes test/scripts/format-generated-module.test.ts, whose Windows pnpm-runner coverage asserts the formatter resolves through cmd.exe /d /s /c ... pnpm.cmd exec oxfmt ... with windowsVerbatimArguments enabled.

Tests and validation

  • node scripts/run-vitest.mjs test/scripts/control-ui-i18n.test.ts test/scripts/format-generated-module.test.ts
  • pnpm ui:i18n:check
  • git diff --check

Risk checklist

  • Keeps formatter command resolution in the existing shared generated-module helper.
  • Preserves the existing direct formatter path on non-Windows when node_modules/.bin/oxfmt exists.
  • No translation content, locale snapshots, dependency, security, auth, or network behavior changes.
  • Highest risk is formatter behavior drift; mitigated by reusing the shared formatter owner instead of adding a second resolver.

Current review state

Ready for ClawSweeper re-review and maintainer review. If type CI is still blocked by the current-main windows-taskkill.mjs declaration gap, rebase after #95527 merges rather than duplicating that declaration in this PR.

@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts size: S labels Jun 21, 2026
@clawsweeper

clawsweeper Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 1:06 PM ET / 17:06 UTC.

Summary
The branch updates scripts/control-ui-i18n.ts so generated locale TypeScript formatting uses scripts/lib/format-generated-module.mjs rather than a local direct-oxfmt/bare-pnpm fallback.

PR surface: Other +7. Total +7 across 1 file.

Reproducibility: yes. I did not rerun Windows locally, but current main and v2026.6.10 still select bare pnpm on the formatter fallback path, and the PR body provides before/after Windows PowerShell pnpm ui:i18n:check evidence.

Review metrics: none identified.

Root-cause cluster
Relationship: canonical
Canonical: #95534
Summary: This PR is the focused formatter-runner fix; the linked Control UI feature PR and Windows taskkill type PR are adjacent validation context, not the same root cause.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Wait for the one still-running exact-head CI job to finish before merge.

Risk before merge

  • [P1] Exact-head CI still had one in-progress CI job in live status, so merge should wait for the normal required checks to finish.

Maintainer options:

  1. Decide the mitigation before merge
    Land the focused shared-formatter change after exact-head checks finish, keeping formatter command resolution owned by scripts/lib/format-generated-module.mjs.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] No repair job is needed because no actionable patch defect was found; maintainers should wait for exact-head CI and then review or merge normally.

Security
Cleared: The diff only changes local script formatter routing to an existing helper and does not alter dependencies, workflows, secrets, lockfiles, or package metadata.

Review details

Best possible solution:

Land the focused shared-formatter change after exact-head checks finish, keeping formatter command resolution owned by scripts/lib/format-generated-module.mjs.

Do we have a high-confidence way to reproduce the issue?

Yes. I did not rerun Windows locally, but current main and v2026.6.10 still select bare pnpm on the formatter fallback path, and the PR body provides before/after Windows PowerShell pnpm ui:i18n:check evidence.

Is this the best way to solve the issue?

Yes. Reusing scripts/lib/format-generated-module.mjs is the narrow maintainable fix because that helper already owns direct oxfmt selection plus the Windows pnpm-runner fallback.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1cd6f81a46ae.

Label changes

Label justifications:

  • P2: This fixes a bounded Windows failure in a repository i18n script with limited product-runtime blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-fix Windows PowerShell live output for pnpm ui:i18n:check completing all locales, plus before-failure context for spawn pnpm ENOENT.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix Windows PowerShell live output for pnpm ui:i18n:check completing all locales, plus before-failure context for spawn pnpm ENOENT.
Evidence reviewed

PR surface:

Other +7. Total +7 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 18 11 +7
Total 1 18 11 +7

What I checked:

Likely related people:

  • Vincent Koc: Recent commits authored related Windows script portability, pnpm-runner, and generated-module formatter behavior that this PR reuses. (role: recent area contributor; confidence: high; commits: 282eb74128d9, 6c4028e07369, 43bbde483046; files: scripts/control-ui-i18n.ts, scripts/lib/format-generated-module.mjs, scripts/pnpm-runner.mjs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. labels Jun 21, 2026
@ooiuuii
ooiuuii force-pushed the fix/control-ui-i18n-pnpm-runner branch from a123a0b to 68e97c8 Compare June 21, 2026 09:54
@ooiuuii

ooiuuii commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review finding by reusing the shared generated-module formatter instead of adding a parallel resolver, then reran the requested focused tests and pnpm ui:i18n:check.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. labels Jun 21, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 21, 2026
Merged
@ooiuuii
ooiuuii force-pushed the fix/control-ui-i18n-pnpm-runner branch from 68e97c8 to ae58239 Compare June 26, 2026 16:55
@vincentkoc
vincentkoc merged commit 9d800b7 into openclaw:main Jun 27, 2026
72 checks passed
@github-actions github-actions Bot mentioned this pull request Jun 27, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jun 28, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 28, 2026
@ooiuuii
fix(scripts): route i18n formatter through pnpm runner (openclaw#95534)
669f4f9
This was referenced Jun 28, 2026
Closed
Open
Open
Open
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@ooiuuii @QiuYuang
fix(scripts): route i18n formatter through pnpm runner (openclaw#95534)
0d89afe
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
@ooiuuii chenyangjun-xy
fix(scripts): route i18n formatter through pnpm runner (openclaw#95534)
3770692
This was referenced Jul 2, 2026
Open
Open
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
@ooiuuii @chenyangjun-xy
fix(scripts): route i18n formatter through pnpm runner (openclaw#95534)
40a9ec7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ooiuuii @vincentkoc