Skip to content

fixing locale wip#14

Merged
mlgarchery merged 4 commits into
devfrom
fix-8
Apr 28, 2026
Merged

fixing locale wip#14
mlgarchery merged 4 commits into
devfrom
fix-8

Conversation

@mlgarchery
Copy link
Copy Markdown
Collaborator

@mlgarchery mlgarchery commented Apr 5, 2026

Issue for this PR

Closes #8

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Please provide a description of the issue, the changes you made to fix it, and why they work. It is expected that you understand why your changes work and if you do not understand why at least say as much so a maintainer knows how much to value the PR.

If you paste a large clearly AI generated description here your PR may be IGNORED or CLOSED!

How did you verify your code works?

Screenshots / recordings

If this is a UI change, please include a screenshot or recording.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

If you do not follow this template your PR will be automatically rejected.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 5, 2026

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • "How did you verify your code works?" section is empty. Please explain how you tested.
  • Not all checklist items are checked. Please confirm you have tested locally and have not included unrelated changes.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 5, 2026

Hey! Your PR title fixing locale wip doesn't follow conventional commit format.

Please update it to start with one of:

  • feat: or feat(scope): new feature
  • fix: or fix(scope): bug fix
  • docs: or docs(scope): documentation changes
  • chore: or chore(scope): maintenance tasks
  • refactor: or refactor(scope): code refactoring
  • test: or test(scope): adding or updating tests

Where scope is the package name (e.g., app, desktop, opencode).

See CONTRIBUTING.md for details.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 5, 2026

📝 Walkthrough

Walkthrough

The CLI initialization in packages/opencode/src/index.ts now explicitly sets the yargs locale to "en". Separately, packages/opencode/script/build.ts adds a Bun build plugin (y18n-eunknown-fix) that intercepts y18n Node shim files during Bun.build and rewrites their fs.readFileSync usage so errors with code === "EUNKNOWN" are recoded to ENOENT before being rethrown. No exported/public declarations were changed.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely incomplete; it references issue #8 but lacks concrete details about the problem, the changes made, verification steps, and has unchecked testing checklist items. Fill in the template with specific details: describe the EUNKNOWN error issue, explain the locale/y18n bundle fix, and confirm testing on Windows. Check the testing and unrelated-changes checklist items.
Title check ❓ Inconclusive The title 'fixing locale wip' is vague and incomplete; it uses non-descriptive terms ('wip') and doesn't clearly summarize the specific bug fix being addressed. Replace with a more descriptive title that clearly indicates the fix, e.g., 'Fix Windows CLI crash with EUNKNOWN locale file error' or 'Handle EUNKNOWN errors in y18n locale resolution'.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR changes address the linked issue #8 by setting a default locale and implementing a Bun build plugin to handle EUNKNOWN errors in y18n's file-reading logic.
Out of Scope Changes check ✅ Passed All code changes are directly scoped to fixing the Windows locale error reported in issue #8; no unrelated changes are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-8

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/opencode/src/index.ts`:
- Line 57: The .locale("en") call only changes yargs help language but doesn't
stop y18n from reading locale files; locate where the yargs instance is created
(the code that calls .locale("en") in packages/opencode/src/index.ts) and
prevent y18n filesystem access by replacing or reconfiguring the internal y18n
instance: after creating the yargs instance, access its internal y18n (or
replace it) and initialize it with safe options such as { directory: null } or {
updateFiles: false }, or swap in a no-op i18n implementation so no file reads
occur; ensure the change happens right after the yargs creation where
.locale("en") is set so bundled Windows executables no longer attempt to open
locales.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5fc76fc3-cad3-40c9-be1a-0644c861924d

📥 Commits

Reviewing files that changed from the base of the PR and between 1ac674c and 986ea7e.

📒 Files selected for processing (1)
  • packages/opencode/src/index.ts

Comment thread packages/opencode/src/index.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/opencode/script/build.ts (2)

9-29: String replacement is fragile and lacks validation.

The patch relies on an exact string match of y18n's import statement. If y18n updates this line (even whitespace changes), the replacement silently fails and the Windows crash will resurface. Consider adding a check to warn or fail if the patch didn't apply:

♻️ Proposed improvement to validate the patch
 const y18nPatchPlugin = {
   name: "y18n-eunknown-fix",
   setup(build: { onLoad: Function }) {
     build.onLoad({ filter: /y18n.*platform-shims[/\\\\]node/ }, async (args: { path: string }) => {
       const src = await Bun.file(args.path).text()
+      const target = "import { readFileSync, statSync, writeFile } from 'fs';"
+      if (!src.includes(target)) {
+        console.warn(`[y18n-eunknown-fix] Expected import not found in ${args.path}; patch may be outdated`)
+        return { contents: src, loader: "js" }
+      }
       const patched = src.replace(
-        "import { readFileSync, statSync, writeFile } from 'fs';",
+        target,
         `import { readFileSync as _readFileSync, statSync, writeFile } from 'fs';
 function readFileSync(p, o) {
   try { return _readFileSync(p, o) }
   catch (e) { if (e.code === 'EUNKNOWN') e.code = 'ENOENT'; throw e }
 }`,
       )
       return { contents: patched, loader: "js" }
     })
   },
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/script/build.ts` around lines 9 - 29, The current
y18nPatchPlugin applies a fragile string.replace on the platform-shims file and
doesn't verify the patch succeeded; update the onLoad handler in y18nPatchPlugin
(setup -> build.onLoad) to read the source into src, perform the replacement
into patched, then check if patched differs from src and if not emit a clear
warning or throw an error (including args.path and a hint to update the matcher)
so the build fails fast; if the patch did apply, continue returning { contents:
patched, loader: "js" } as before.

15-16: Use BunPlugin type for improved type safety and IDE support.

The inline type annotations { onLoad: Function } and { path: string } are loosely typed. Since the project already extends @tsconfig/bun, the BunPlugin type from the "bun" module is accessible and provides proper typing for the plugin setup method, as shown in Bun's documentation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/script/build.ts` around lines 15 - 16, Replace the loose
inline types with Bun's proper plugin types: update the setup signature to use
the BunPlugin type for its parameter and use the appropriate Bun type for the
onLoad callback argument (instead of { onLoad: Function } and { path: string }).
Locate the setup method (setup(build: { onLoad: Function })) and the
build.onLoad handler and change their parameter types to the BunPlugin types
imported from "bun" so the setup method and the onLoad callback use the official
Bun types for stronger type safety and IDE support.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/opencode/script/build.ts`:
- Around line 9-29: The current y18nPatchPlugin applies a fragile string.replace
on the platform-shims file and doesn't verify the patch succeeded; update the
onLoad handler in y18nPatchPlugin (setup -> build.onLoad) to read the source
into src, perform the replacement into patched, then check if patched differs
from src and if not emit a clear warning or throw an error (including args.path
and a hint to update the matcher) so the build fails fast; if the patch did
apply, continue returning { contents: patched, loader: "js" } as before.
- Around line 15-16: Replace the loose inline types with Bun's proper plugin
types: update the setup signature to use the BunPlugin type for its parameter
and use the appropriate Bun type for the onLoad callback argument (instead of {
onLoad: Function } and { path: string }). Locate the setup method (setup(build:
{ onLoad: Function })) and the build.onLoad handler and change their parameter
types to the BunPlugin types imported from "bun" so the setup method and the
onLoad callback use the official Bun types for stronger type safety and IDE
support.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e5613389-1684-49d3-938f-fa003a353051

📥 Commits

Reviewing files that changed from the base of the PR and between 986ea7e and a2e6c2d.

📒 Files selected for processing (1)
  • packages/opencode/script/build.ts

@Zenwoh Zenwoh self-assigned this Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Windows] CLI crashes immediately: EUNKNOWN open 'B:\~BUN\locales\en_US.json'

2 participants