Android: Hermes release builds on Windows hosts (Task 13.7, BLK-07)

[krassx]

Fixes release blocker BLK-07 / Task 13.7: a Hermes release build of an app integrated per the README (bare or Expo) could not be built on a Windows host.

Root cause (#61, runs 37566757670, 37567458498, 37584777337, 37587420367): react.hermesCommand pointed at scripts/hermesc-preserve-js.sh. On Windows, React Native runs that command through cmd /c (BundleHermesCTask.getHermescCommand → windowsAwareCommandLine; detectOSAwareHermesCommand keeps a user-set command as is). cmd can't run a bash script, so the step exited 0 without writing bytecode, and the build failed later with NoSuchFileException on index.android.bundle.hbc.

Design

• The wrapper's logic moves into scripts/hermesc-preserve-js.js (Node). Two thin launchers start it: hermesc-preserve-js.sh for macOS and Linux (builds already wired to it keep working) and hermesc-preserve-js.cmd for Windows (CRLF line endings, pinned in .gitattributes). hermesc is found the same way as before, using win64-bin/hermesc.exe on Windows.
• hermesCommand picks the launcher when Gradle configures the build: System.getProperty("os.name").startsWith("Windows") ? ".cmd" : ".sh". One build.gradle works for a repo shared across macOS, Linux and Windows. I chose this over relying on cmd's extension lookup (PATHEXT) for an extensionless file, because nothing then depends on how cmd resolves names.
• The wrapper fails loudly on every OS. It exits non-zero when hermesc exits 0 but writes no bytecode or an empty -out (a stale -out file is deleted first), when hermesc is killed, when it can't start, or when it gets no -out.
• bugsee-sourcemaps.gradle guard: on Windows, a Hermes bundle task whose hermesCommand names a .sh file fails before hermesc runs, and the error gives the line to use. This covers apps wired per the earlier README.
• Expo plugin: it writes the per-OS expression. It recognises the exact line an earlier prebuild wrote (the .sh on every OS) and moves it to the per-OS one. A user's own hermesCommand that already names hermesc-preserve-js is left as it is. A setting that doesn't name the wrapper is rewritten or refused, as before.

Tests

• scripts/__tests__/hermesc-preserve-js.test.js (new): the script runs in-process for mutation testing, plus the launcher this host uses. On Windows that is the .cmd, run through cmd /c with the repository's hermesc.exe.
• Gradle corpus: 11 new cases covering migration (LF, CRLF, tab indentation, trailing comment) and lines that must stay as written (a user .sh, a user .cmd, the old expression with more after it, in .set(…), after ;, inside a comment, in another block).
• Real-Gradle hook test: a Hermes task whose hermesCommand names a .SH is not refused off Windows. The "missing preserve" message now names the per-OS fix.
• stryker.plugin.json now mutates hermesc-preserve-js.js.
• Windows workflow: added the wrapper tests on Windows, a negative step for the .sh guard, and an Expo job (prebuild on the runner, release build, debug id in the composed map and in the bytecode, upload to the stub).

Based on #61, which adds the windows-latest workflow. I'll retarget to main once #61 merges.

🤖 Generated with Claude Code

[cursor]

Stale comment

Review summary

This is a first review of a9ef177 (Windows Hermes release: Node wrapper, .cmd/.sh launchers, per-OS hermesCommand, Gradle .sh guard, Expo migration).

The product fix is sound. React Native 0.87 detectOSAwareHermesCommand keeps a user-set command and only rewrites / to the platform separator — it does not append .exe, so a .cmd launcher is the right shape. Fail-loud on empty bytecode, the Windows .sh doFirst guard, and the exact-text Expo migration match the BLK-07 failure mode.

One real break on this branch: the README groovy snippet is now two lines, and campaign gen-rn-app still copies only the first.

Did not run Jest (node_modules is not present in this environment). CI was still pending at review time.

Findings

1. P1 packages/react-native/README.md (consumed by scripts/campaign/lib/readme-android.js) — two-line hermesCommand is truncated into invalid Groovy.

Verdict

1. Overall risk: Medium
2. Merge recommendation: Request changes
3. Must fix: Restore a single-line README hermesCommand (or teach readme-android.js to take the continuation) and assert the injected line still contains .cmd / .sh.
4. What works: Node wrapper + thin launchers, stale -out deletion, corpus cases for legacy Expo lines, .gitattributes CRLF on .cmd, Windows job covering wrapper tests / the .sh guard / Expo prebuild.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Review summary

Re-review of d8d2eda (Windows Hermes release: Node wrapper, .cmd/.sh launchers, per-OS hermesCommand, Gradle .sh guard, Expo migration). Prior finding was on 0b830fb / a9ef177.

The previous P1 is fixed. The README hermesCommand is a single Groovy assignment that names both launchers, readme-android.js now refuses a value whose brackets do not close on that line or that omits .cmd/.sh, and scripts/__tests__/campaign-readme-android.test.ts runs the real README through the extractor. Confirmed by executing that same one-line match against packages/react-native/README.md (bracket depth 0, both launchers present).

No new P0–P2. Checked against surrounding code, not the diff alone:

• React Native 0.87 detectOSAwareHermesCommand keeps a user-set command and only rewrites / to the platform separator — it does not append .exe, so a .cmd launcher is the right shape for cmd /c.
• The wrapper deletes a stale -out and exits non-zero when hermesc writes nothing; package.json files includes the .js, .sh, and .cmd; the .cmd in tree is CRLF.
• The Windows doFirst guard (endsWith(".sh")) is the intended path for a leftover README File(..., "hermesc-preserve-js.sh") line, which Expo correctly leaves alone (not the exact legacy concat expression).

Did not run Jest (node_modules is not present). Windows campaign jobs for this HEAD were still queued at review time.

Findings

None remaining.

Verdict

1. Overall risk: Low
2. Merge recommendation: Approve
3. Must fix: Nothing. The README / gen-rn-app truncation is addressed.
4. What works: One-line per-OS hermesCommand in the README fence, extractor fail-closed on a split value, Node wrapper + thin launchers, exact-text Expo migration, .gitattributes CRLF on .cmd.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Review summary

Re-review of d8d2eda (Windows Hermes release: Node wrapper, .cmd/.sh launchers, per-OS hermesCommand, Gradle .sh guard, Expo migration).

The earlier P1 is fixed: the README hermesCommand is one closed Groovy line, readme-android.js refuses a split value, and campaign-readme-android.test.ts runs the real README through the extractor.

The product path looks right. React Native 0.87 detectOSAwareHermesCommand keeps a user-set command and only rewrites / to the platform separator — it does not append .exe, so a .cmd launcher is the correct shape. On windows-latest job 112694086548, 37 of 38 wrapper tests passed, including the .cmd through cmd /c with hermesc.exe. Expo prebuild on the sibling Windows job also succeeded.

The remaining break is the one assertion 54df3b4 did not convert to realpathSync.native. That failure is a hard gate in campaign-windows-hermes.yml, so the bare Windows assembleRelease (the BLK-07 proof) never ran on this commit.

Findings

1. P1 packages/react-native/scripts/__tests__/hermesc-preserve-js.test.js — resolveFrom still compares realpathSync paths; Windows CI dies on 8.3 vs long form and skips the release build.

Verdict

1. Overall risk: Medium
2. Merge recommendation: Request changes
3. Must fix: Use fs.realpathSync.native on both sides of the resolveFrom assertion (same as the findHermesc test at line 161). Re-run the Windows wrapper-test job and let assembleRelease proceed.
4. What works: One-line README + extractor guard; .cmd via cmd /c; fail-loud empty bytecode; Windows .sh doFirst guard; Expo exact-text migration of the old .sh expression.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Review summary

Re-review of 670c696 (Windows Hermes release: Node wrapper, .cmd/.sh launchers, per-OS hermesCommand, Gradle .sh guard, Expo migration, pwsh guard-step exit). Prior findings were on c4c844b / d8d2eda / 0b830fb.

Both earlier P1s are fully_addressed:

• README hermesCommand is one closed Groovy line that names both launchers; readme-android.js refuses a split value. Confirmed by running the same one-line match against packages/react-native/README.md (bracket depth 0, .cmd and .sh present).
• resolveFrom compares fs.realpathSync.native on both sides. On windows-latest job 112699789721 (c4c844b), wrapper tests, :app:assembleRelease, composed-map debug id, bytecode id, and stub PUT all succeeded.

The remaining red on that job was the negative .sh guard step, not the product path. PowerShell keeps Gradle's $LASTEXITCODE after Write-Output; 670c696 exits 0 once the three fail-closed checks pass. That matches the log (guard: failed with the fix, before hermesc then a red step).

Checked against surrounding code, not the diff alone:

• React Native 0.86.3 and 0.87.1 detectOSAwareHermesCommand keep a user-set command and only rewrite / to File.separatorChar. They do not append .exe. windowsAwareCommandLine is cmd /c plus the .cmd path and hermesc args, which is the shape the launcher tests already run.
• The wrapper deletes a stale -out and exits non-zero when hermesc writes nothing. package.json files includes .js, .sh, and .cmd. The .cmd in tree is CRLF (\r only).
• Expo migrates the exact legacy concat expression; a leftover README File(..., "hermesc-preserve-js.sh") is left as written and is the Windows doFirst endsWith(".sh") path.

Did not run Jest (node_modules is not present). The Windows campaign run for 670c696 was still in progress at review time; the BLK-07 proof already landed on c4c844b.

Findings

None remaining.

Verdict

1. Overall risk: Low
2. Merge recommendation: Approve
3. Must fix: Nothing. Prior P1s are closed; 670c696 only corrects the guard step's pwsh exit code.
4. What works: Node wrapper + thin launchers; one-line README + extractor fail-closed; .cmd via cmd /c with hermesc.exe on windows-latest; fail-loud empty bytecode; Windows .sh doFirst guard; Expo exact-text migration of the old .sh expression.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Review summary

Re-review of edd126f (new android-release-spaced-path job). Prior P1s on d8d2eda / c4c844b remain fully_addressed (one-line README hermesCommand + extractor guard; resolveFrom uses realpathSync.native). Product path on a space-free Windows checkout was already proven on c4c844b (wrapper tests, :app:assembleRelease, composed-map debug id, bytecode id, stub PUT).

The new job does not prove cmd /c quoting. On run 37600186416, job 112722382424 it died in CMake/ninja on CMAKE_OBJECT_PATH_MAX before any createBundle* / hermesc-preserve-js ran. Yarn install, credentials, and the stub ({"port":8777}) succeeded.

Checked against surrounding code, not the diff alone:

• RN 0.87.1 windowsAwareCommandLine is cmd /c + args; detectOSAwareHermesCommand keeps a user-set command and only rewrites / → File.separatorChar (no cliPath). Default hermesc (unset hermesCommand) uses File.cliPath, which is relative on Windows.
• This repo still assigns file(...).absolutePath / getAbsolutePath() + "/scripts/hermesc-preserve-js" + …. That is untested here because CMake failed first. If a short spaced path ever reaches hermesc, that absolute path is the next thing to watch (cmd /c treats () inside the quoted command as special).
• README still says Windows builds are verified without spaces, which matches the red job.

Did not run Jest (node_modules is not present). The failure above is from the GitHub Actions log for this HEAD, not from a local Windows run.

Findings

1. P1 .github/workflows/campaign-windows-hermes.yml — checkout my projects (x86)/bugsee rn exceeds CMAKE_OBJECT_PATH_MAX; the job never exercises Hermes/cmd /c.

Verdict

1. Overall risk: Medium
2. Merge recommendation: Request changes
3. Must fix: Shorten the spaced checkout (e.g. p (x)/a) so New Architecture object paths stay under 250 characters, and confirm :app:createBundleReleaseJsAndAssets actually runs. Drop the job if a short path still cannot host codegen. Do not treat this red X as a Hermes quoting result.
4. What works: Node wrapper + .cmd/.sh launchers on a space-free windows-latest path; one-line README + extractor fail-closed; Windows .sh doFirst guard; Expo exact-text migration of the old .sh expression.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Review summary

Re-review of c258fb3. The P1 from edd126f is fully_addressed: the spaced-path job no longer checkouts under the workspace. It copies to C:/s (x86)/b rn.

The FeedbackSpec object directory that was 228 characters (cap 250) on job 112722382424 becomes 171 with that prefix. The longest New Architecture object path I measured (ComponentDescriptors.cpp.o under FeedbackSpec) is 242 ≤ 250. That is the CMake failure from the last push, not a Hermes/cmd /c result.

No new P0–P2. The campaign Windows run for this HEAD was still in progress at review time, so this does not claim :app:createBundleReleaseJsAndAssets has succeeded yet — only that the path-length blocker in the workflow is gone. hermesCommand is still .absolutePath (example, README, Expo plugin). RN 0.87 keeps a user-set command as-is and prefixes cmd /c; that remains the thing to watch if the short root reaches hermesc and it fails.

Findings

None remaining.

Verdict

1. Overall risk: Low
2. Merge recommendation: Approve
3. Must fix: Nothing in the workflow text. Confirm the spaced-path job gets past CMake and actually runs the bundle task.
4. What works: Short C:/s (x86)/b rn root keeps spaces and (); yarn install runs in the copy; space-free Windows assembleRelease was already green on c4c844b.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Review summary

Re-review of d5842db. c258fb3 got past CMake on C:/s (x86)/b rn, then createBundleReleaseJsAndAssets failed with 'C:\\s' is not recognized (run 37601601358). That is the cmd /c quote rule: spaces plus () in an absolute hermesCommand make cmd drop the quotes and cut at the first space. RN 0.87 windowsAwareCommandLine is cmd /c + the command; detectOSAwareHermesCommand keeps a user-set path as-is; default hermesc uses File.cliPath (relative on Windows) and exec.workingDir(root).

This commit does the same for Bugsee: on Windows, bugsee-sourcemaps.gradle rewrites an absolute react.hermesCommand that would be cut to a path relative to react.root, unless the relative form would still be cut or the files are on different drives. RN TaskConfiguration does task.hermesCommand.set(config.hermesCommand) (Property as Provider), so the task sees the rewritten value. The .sh doFirst guard still matches a relative *.sh. Artifact upload is split (APK on C:, logs on D:).

No new P0–P2. The campaign Windows run for d5842db was still in progress at review time; this is a code review of the rewrite, not a claim that the spaced-path job is green yet.

Findings

None remaining.

Verdict

1. Overall risk: Low
2. Merge recommendation: Approve
3. Must fix: Nothing. Let the spaced-path job confirm createBundleReleaseJsAndAssets on C:/s (x86)/b rn.
4. What works: Relative launcher for cmd /c; CMake-safe short root; space-free Windows assembleRelease already green on c4c844b; prior README / realpathSync.native / guard-exit P1s still closed.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Review summary

Re-review of d5842db. Prior P1s remain fully_addressed: one-line README hermesCommand + extractor guard; resolveFrom uses realpathSync.native; spaced-path job uses C:/s (x86)/b rn and gets past CMake.

The new hook is the right fix for the failure that followed. On run 37601601358, job 112727053095 (c258fb3), :app:createBundleReleaseJsAndAssets died with 'C:\s' is not recognized — cmd /c stripped quotes around the absolute .cmd because the path holds spaces and (). That is the BLK-07 quoting case, not CMake.

Checked against surrounding code, not the diff alone:

• RN 0.81 and 0.87 File.cliPath already returns a path relative to react.root on Windows (workingDir is that root). detectOSAwareHermesCommand keeps a user-set command as-is (only / → File.separatorChar) and does not call cliPath, which is why .absolutePath broke.
• task.hermesCommand.set(config.hermesCommand) is a live Provider, so rewriting the extension at apply from time is what the bundle task runs. Relativize uses normalize(), so android/app/../../node_modules/… becomes node_modules\… with no spaces or ().
• Empty default hermesCommand ("") is present but not absolute, so the rewrite no-ops and RN’s own relative autodetect stays.
• Another drive, or a relative path that still matches [\s&<>()@^|], is left as written (documented).

Did not run Jest (node_modules is not present). Wrapper tests on this HEAD already passed on windows-latest (run 37602820088). The spaced-path assembleRelease for d5842db was still in progress at review time; this does not claim that job is green yet — only that the quoting rewrite matches RN’s own path handling and the 'C:\s' failure mode.

Findings

None remaining.

Verdict

1. Overall risk: Low
2. Merge recommendation: Approve
3. Must fix: Nothing. Confirm the spaced-path job’s :app:createBundleReleaseJsAndAssets succeeds on this HEAD.
4. What works: Node wrapper + .cmd/.sh launchers; fail-loud empty bytecode; Windows .sh doFirst guard; Expo exact-text migration; one-line README + extractor fail-closed; hermesCommand relativized from react.root when cmd /c would cut the absolute path.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Review summary

Re-review of c34ceba. d5842db relativized hermesCommand on Windows, then rejected that relative path because node_modules\\@bugsee\\... contains @. @ is special only inside cmd’s quoted-path rule (&<>()@^|). An unquoted relative command is split by space or & < > ^ |, not by @. The fallback kept the absolute launcher, so run 37602820088 still executed C:\\s (x86)\\... and cmd /c cut it at C:\\s.

This commit narrows bugseeCmdUnsafe to [\\s&<>^|]. The example/README/Expo absolute path still has spaces, so it is still rewritten; node_modules\\@bugsee\\... is now accepted. RN 0.87 task.hermesCommand.set(config.hermesCommand) remains a live Provider.

No new P0–P2. The campaign Windows run for c34ceba was still queued at review time.

Findings

None remaining.

Verdict

1. Overall risk: Low
2. Merge recommendation: Approve
3. Must fix: Nothing. Confirm the spaced-path job’s createBundleReleaseJsAndAssets on this HEAD.
4. What works: Relative launcher with a scoped @bugsee segment; CMake-safe C:/s (x86)/b rn root; prior P1s (README line, realpathSync.native, MAX_PATH checkout) still closed.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Review summary

Re-review of c34ceba. Prior P1s remain fully_addressed: one-line README hermesCommand + extractor guard; resolveFrom uses realpathSync.native; spaced-path job uses C:/s (x86)/b rn.

The @ diagnosis from run 37602820088, job 112731270419 (d5842db) is right. Relativize produced node_modules\@bugsee\..., the old bugseeCmdUnsafe ([\s&<>()@^|]) rejected that relative form, and the absolute launcher still died with 'C:\\s' is not recognized. @ is only special inside cmd's quoted /c rule. Java's ProcessImpl CMD/WIN32 escape set is space/tab/"<>&|^ — it does not quote @ — so an unquoted relative @bugsee path is the correct shape. RN 0.81/0.87 detectOSAwareHermesCommand still keeps a user-set command as-is (only / → File.separatorChar) and BundleHermesCTask still runs with workingDir(root).

Dropping @ from the shared regex is the right fix for the campaign path (spaces + (x86)). Dropping () along with it is not. The default relative launcher never contains parentheses; putting () back would still accept node_modules\@bugsee\... and would still rewrite C:\\Users\\John(US)\\....

Findings

1. P2 packages/react-native/scripts/bugsee-sourcemaps.gradle — rewrite is skipped when the absolute path has () but no space, so C:\\Users\\John(US)\\... keeps the absolute .cmd.

Verdict

1. Overall risk: Medium
2. Merge recommendation: Request changes
3. Must fix: Restore () in the rewrite/reject set (only @ had to leave), or always relativize when the relative form is unquoted-safe ([\s&<>()^|], not @). Add a table test of bugseeCmdSafeCommand so the next charset edit cannot re-break @bugsee or skip ().
4. What works: @ no longer blocks the relative launcher; Node wrapper + .cmd/.sh; fail-loud empty bytecode; Windows .sh doFirst guard; Expo exact-text migration; one-line README + extractor fail-closed; space-free windows-latest assembleRelease already green on c4c844b.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Review summary

Re-review of 420f41a. The P2 on c34ceba (one regex dropping (), so C:\\Users\\John(US)\\… never relativized) is fully_addressed. Absolute paths that hold a space or ( ) & < > @ ^ | are rewritten; the relative form is used unless it holds a space or ( ) & < > ^ | (@ allowed unquoted). That is the right split: @ is only special inside cmd’s quoted /c rule; unquoted ( ) still group.

The real-Gradle hook test now drives bugseeCmdSafeCommand on the campaign path (space + ( ) + @bugsee), John(US) with no space, a plain @bugsee tree, &, two trees whose relative form would still be cut, and an already-relative path. Author reports run 37604429856 on c34ceba green (debug id in map, bytecode, and stub); this HEAD adds coverage for the no-space parens case that job could not see.

No new P0–P2. RN 0.87 task.hermesCommand.set(config.hermesCommand) remains a live Provider. Prior P1s (README line, realpathSync.native, CMAKE_OBJECT_PATH_MAX checkout) remain closed.

Findings

None remaining.

Verdict

1. Overall risk: Low
2. Merge recommendation: Approve
3. Must fix: Nothing.
4. What works: Relative launcher for cmd /c including scoped @bugsee and John(US); CMake-safe C:/s (x86)/b rn; Gradle table for the rewrite; Node wrapper + .cmd/.sh; Windows .sh doFirst guard; Expo exact-text migration; one-line README + extractor fail-closed.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Review summary

Re-review of 420f41a. Prior findings remain fully_addressed: one-line README hermesCommand + extractor guard; resolveFrom uses realpathSync.native; spaced-path job uses C:/s (x86)/b rn.

The previous P2 is fully_addressed. bugseeCmdSafeCommand now has two classes: rewrite an absolute path that holds a space or ( ) & < > @ ^ |, and accept the relative form unless it holds a space or ( ) & < > ^ | (@ stays allowed unquoted). That is the split that c34ceba needed: node_modules\@bugsee\... is kept, and C:\Users\John(US)\... (no space, so Java never quotes it) is rewritten before cmd groups on (US).

Checked against surrounding code, not the diff alone:

• The Gradle helper table covers the campaign path, John(US), a scoped @bugsee path with no other specials, a path cmd would not cut, &, a relative that would still be cut, and an already-relative path. I walked each case against the two regexes; they match the expected map in sourcemaps-gradle.test.js.
• RN 0.81/0.87 still keep a user-set hermesCommand (only / → File.separatorChar) and run it via windowsAwareCommandLine (cmd /c + args) with workingDir(root). Relativizing against react.root is the same rule RN uses for its own paths. task.hermesCommand.set(config.hermesCommand) remains a live Provider, so the rewrite at apply from time is what the bundle task runs.
• Campaign run 37604429856 on c34ceba already succeeded for space-free Windows assembleRelease, Expo prebuild+release, and the spaced-path job. 420f41a does not change that relative @bugsee shape; it only extends the absolute trigger to () without a space.

Did not run Jest (node_modules is not present). Confirmed the committed .cmd is CRLF-only and HERMES_COMMAND_EXPR matches between plugin/src/gradle.ts and plugin/build/gradle.js. The Windows campaign for 420f41a was still in progress at review time.

Findings

None remaining.

Verdict

1. Overall risk: Low
2. Merge recommendation: Approve
3. Must fix: Nothing. The () hole is closed; @bugsee remains unquoted-safe.
4. What works: Split rewrite/reject sets with a table test; Node wrapper + .cmd/.sh launchers; fail-loud empty bytecode; Windows .sh doFirst guard; Expo exact-text migration; one-line README + extractor fail-closed; spaced-path assembleRelease already green on c34ceba.

  

_{Sent by Cursor Automation: Bugsee code review}