chore(browser-runtime): make sync.mjs runnable on Windows - #1834
Conversation
The vendoring sync script assumed POSIX-only paths: - msys GNU tar treats `C:` drive letters as remote hosts and requires --wildcards for glob extraction; quoted Windows paths are converted to /c/... and tar gets --wildcards - path.join emits '\' on win32, so LOCAL_PATCHES keys and the leaf 'src/' prefix filter never matched; normalize to POSIX separators No generated artifacts change: contentHash stays identical to the lock. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
The sync.mjs Windows compatibility fixes moved to their own PR (makecindy#1834); this PR keeps only the LOCAL_PATCHES allowedOrigins wiring. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
|
| Filename | Overview |
|---|---|
| scripts/browser-runtime/sync.mjs | Windows 路径、shell 与 tar 兼容处理已补齐；既有 tar 相关评审问题在当前代码中均已修复。 |
Reviews (7): Last reviewed commit: "fix: use English for the sh-resolution e..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟢 Ready to approve
变更范围集中在本地同步脚本的 Windows 兼容处理，且不涉及产物/运行时代码路径的行为变更。
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
该 PR 修复 scripts/browser-runtime/sync.mjs 在 Windows（git bash/msys）下不可运行的问题，使维护者能够在 Windows 上重新生成并校验 packages/browser-control-runtime/src/_generated/**（且不改变生成产物）。
Changes:
- 在
shHide()中对 msys 环境做兼容：将双引号内的 Windows 盘符路径转换为/c/...形式，并为tar注入--wildcards。 - 统一路径分隔符为 POSIX（
/），避免 win32 下path.join/path.relative产生的反斜杠导致 leaf 过滤与LOCAL_PATCHESkey 匹配失败。
File summaries
| File | Description |
|---|---|
| scripts/browser-runtime/sync.mjs | 修复 Windows(msys) 下 tar/路径分隔符行为，保证同步脚本可在 Windows 正常运行且保持生成结果不变。 |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b70b977ae
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Greptile P1 + Codex P2: unconditionally injecting GNU tar's --wildcards broke macOS maintainers' BSD/libarchive tar. The whole msys compatibility path now runs only on win32; POSIX platforms keep the upstream command byte-for-byte. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
|
@happy9zhang 👋 这个 PR 还有 2 条 review conversation 没 resolve(scripts/browser-runtime/sync.mjs),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
MagicLizi
added
status:ci-failed
There was a problem hiding this comment.
🟢 Ready to approve
改动范围局限于本地同步脚本的 Windows 兼容处理，且 POSIX 平台路径保持不变的分支明确，未发现会影响现有生成逻辑与 lock 一致性的风险点。
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
MagicLizi
added
status:ci-running
|
@happy9zhang 👋 这个 PR 还有 2 条 review conversation 没 resolve(scripts/browser-runtime/sync.mjs),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
On win32 the PATH may resolve to the system bsdtar/libarchive instead of msys GNU tar; bsdtar treats unsupported options as fatal errors, so the unconditional --wildcards injection aborted pnpm sync:browser-runtime. Detect the tar implementation via `tar --version` (cached) and inject --wildcards only for GNU tar; bsdtar matches extraction paths as shell-style patterns natively, so no flag is needed there. Greptile P1 round 1 (Windows bsdtar) / Codex P2 (BSD tar) — covers both. Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
win32 下对 JSON.stringify(...) 产生的 quoted 路径直接做反斜杠替换会生成双斜杠 msys 路径（如 /c//Users//...），存在路径解析不一致风险，需要先解码 quoted string 再转换。
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
scripts/browser-runtime/sync.mjs:92
- win32 分支里对双引号参数做路径转换时直接处理捕获到的文本
p；但这里的路径参数目前都来自JSON.stringify(...)，其中每个\在命令文本里会被转义成\\。当前toMsys(p)会把这些转义反斜杠也当作路径分隔符替换成/，从而生成类似/c//Users//...的双斜杠路径，存在被 msys/工具链按 UNC 或异常路径解析的风险。建议先把匹配到的 quoted string 用JSON.parse解码回原始路径，再做toMsys，最后再JSON.stringify回去。
const wildcards = isGnuTar() ? ' --wildcards' : '';
const toMsys = (p) => p.replace(/^([A-Za-z]):[\\/]/, (_, d) => `/${d.toLowerCase()}/`).replace(/\\/g, '/');
const posix = cmd
.replace(/"((?:[A-Za-z]:)?[^"]*)"/g, (m, p) => JSON.stringify(toMsys(p)))
.replace(/(^|\s)tar(\s)/g, `$1tar${wildcards}$2`);
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟢 Ready to approve
变更范围小且均有 win32 条件分支/幂等归一化保护，未发现会影响 POSIX 行为或改变生成产物的逻辑问题。
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b5d523542
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
回复 Copilot review（Changes recommended，2026-08-06T10:26:56Z）：已修复（ |
|
@happy9zhang 👋 这个 PR 还有 1 条 review conversation 没 resolve(scripts/browser-runtime/sync.mjs),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
MagicLizi
added
status:threads-open
On win32 the PATH may only contain Git\cmd (not Git\bin), so a bare `sh` would ENOENT and abort pnpm sync:browser-runtime. Resolve the Git-bundled sh.exe via scripts/lib/posix-shell.mjs's resolvePosixShell (where.exe derivation + Program Files / LOCALAPPDATA standard locations; non-win32 keeps PATH behavior). Fail with a clear install-Git message when no sh is found (codex-connector P1, round 3). Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
新增的错误提示文本与脚本其余输出语言不一致，需要调整以保持可维护性与一致的可读性。
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
scripts/browser-runtime/sync.mjs:90
- 这里新增的错误信息为中文，但该脚本其余错误/日志基本为英文；在跨区域维护（尤其是脚本类工具）场景下会造成体验不一致。建议改为英文（或至少中英双语）并明确这是 Windows 需要 Git for Windows 提供的 sh。
if (!sh) {
throw new Error(
'sync: 找不到可用的 sh（未检测到 Git for Windows）。请安装 Git for Windows 后重试。',
);
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4e43f448d
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
On win32 the Node process PATH may resolve `tar` to System32\bsdtar while
the Git sh resolves GNU tar — probing with execFileSync('tar') could then
report non-GNU and omit --wildcards, while the actual GNU tar in the sh
environment would treat quoted member patterns literally and abort the
extraction. isGnuTar() now probes `tar --version` through the resolved sh
(the same environment that runs the tar commands), keeping detection and
execution consistent (Greptile P1 + codex-connector P1, round 4).
Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Ready to approve
变更聚焦于 Windows 兼容性修复且未触及生成逻辑核心路径，发现的问题仅为错误信息语言一致性的小修正。
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
scripts/browser-runtime/sync.mjs:99
- 这里的错误信息是中文，而同文件内其他错误信息基本都是英文（例如 ghPreflight / parseGithubRepo 等），会导致脚本输出语言不一致、对非中文维护者不友好。建议改为英文或中英双语保持一致性。
throw new Error(
'sync: 找不到可用的 sh（未检测到 Git for Windows）。请安装 Git for Windows 后重试。',
);
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
The "could not locate a usable sh" error text was the only Chinese output in the script; align it with the rest of the output language for maintainability and readability (Copilot P1 round 5). Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
|
回复 Copilot review（Changes recommended，2026-08-06T11:57:53Z）：已修复（ |
There was a problem hiding this comment.
🟢 Ready to approve
变更范围小且聚焦于 Windows 兼容性处理，关键逻辑（POSIX 平台 no-op、GNU tar 探测与分隔符归一化）自洽，未发现会影响既有行为或产物一致性的明确问题。
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
MagicLizi
added
status:awaiting-bot-review
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
Light review passed. Windows compatibility changes are well-structured: path normalization, GNU/bsd tar detection, and sh resolution via the existing posix-shell helper. No security or correctness concerns.
|
Making scripts cross-platform without breaking the POSIX path is trickier than it looks — the GNU vs bsd tar detection and the msys path rewriting are both done carefully here. Thanks for the solid fix! 🛠 |
这次改了什么
摘要
scripts/browser-runtime/sync.mjs（vendored 同步脚本）此前在 Windows 上无法运行，导致维护者在 Windows 上无法重新生成_generated/**：C:盘符路径当成远程主机（host:path），且默认关闭通配符匹配。修复为把引号内的 Windows 路径转换为/c/...形式并给 tar 传--wildcards。path.join在 win32 上输出反斜杠，导致LOCAL_PATCHES的 key（POSIX 正斜杠）永远匹配不上、leaf 的src/前缀过滤把全部 leaf 文件过滤掉。修复为统一转成 POSIX 分隔符。关键性质：不改变任何生成产物——重新生成后
contentHash与既有 lock 完全一致（--check通过），无_generated/ lock 变更。变更类型
chore工程维护范围
scripts/browser-runtime/sync.mjs3 处 Windows 兼容修复（无产物变化）previewLocalHtml功能与 SSRF allowedOrigins 接线（在 PR feat(browser): add sandboxed local HTML preview #1803）UI 变化
不涉及：无 UI 代码路径。
怎么验证的
自动验证
Windows 10（git bash/msys）实测可完整跑通
sync.mjs（下载 tarball → 解压 → 应用 LOCAL_PATCHES → 生成 335 个文件）。手工验证
不涉及。
未执行的验证
无。
风险
风险分类
影响与回滚
sync.mjs在 Windows 本地运行；CI（Linux）路径行为不变（修复对 POSIX 路径为 no-op）提交前检查
git commit -s）