Stabilize Windows Codex flow and gate cookie sync#419
Merged
Conversation
Three Windows-only gaps showed up in real dev testing. Cookie sync silently returned 0 cookies because Chrome 127+ App-Bound Encryption binds keys to the original user-data-dir (a copied profile decrypts to empty), and Chrome 136+ refuses --remote-debugging-port on the default profile, so neither copy-then-launch nor launch-against-original returns plaintext cookies. The provider-step engine installer ran npm through cmd.exe in ways that have been unreliable on real machines. The vendored browser-harness-js script has no Windows extension, so Codex's Rust spawn path falls through to ShellExecuteW and triggers the OS "How do you want to open this file?" dialog every time the model emits a browser-harness-js call. Skip the cookie sync UI on Windows: the onboarding profile step and the Settings Browser Sync section render only on macOS/Linux. Replace the provider-step background install on Windows with a copy-command flow that puts the npm command on the clipboard and polls detect-IPC until the user has run it manually. Fix shim resolution so .cmd wins the PATHEXT tiebreaker over .ps1 (cmd.exe avoids the powershell.exe ENOENT and cold-start observed inside Electron's main process), and route .ps1-only packages through the absolute %SystemRoot%\System32\WindowsPowerShell\ v1.0\powershell.exe path so .ps1 fallback no longer depends on env.Path lookup. Add browser-harness-js.cmd alongside the bash CLI so Codex's CreateProcessW finds a PATHEXT-resolvable entry that delegates to Git Bash and keeps Claude Code's existing bash-direct path untouched. Constraint: macOS and Linux flows must remain byte-identical -- the existing copy-then-launch cookie sync, the background install spawn, and the bare-name harness CLI invocation all keep working on POSIX. Constraint: The .cmd shim must fail cleanly when Git for Windows is missing -- exits 1 with a stderr hint instead of silently regressing into the OS popup. Rejected: Implement Chrome v20 ABE decryption (DPAPI outer + IElevator COM inner) | real native code, brittle to Chromium updates, larger than this branch should carry; Windows is hidden until that lands. Rejected: Always prefer .ps1 over .cmd in the resolver | leaves the Electron-main powershell.exe ENOENT failures in place even when a working .cmd shim is sitting next to it. Rejected: Auto-install Codex/Claude in the background on Windows | cmd.exe quoting around npm shims has been observed to hang or trigger UAC prompts in real installs; manual run + clipboard handoff is more predictable. Confidence: high Scope-risk: narrow Directive: Keep cookie sync hidden on Windows until ABE decryption ships. The onboarding step and Settings tab are gated on window.onboardingAPI.platform === 'win32' and window.electronAPI.shell.platform === 'win32' respectively. Directive: When adding new shim-resolved CLIs that ship .cmd plus .ps1, trust the existing pathEnrich tiebreaker -- do not reintroduce a .ps1-first preference without revisiting the powershell.exe spawn issue. Tested: npm run typecheck Tested: npm run lint Tested: npx vitest run tests/unit/hl/harnessBootstrap.test.ts tests/unit/hl/codexStdinWindows.test.ts tests/unit/pathEnrich.test.ts tests/unit/identity/codexLogin.test.ts tests/unit/onboarding/OnboardingApp.spec.tsx tests/unit/renderer/ipcErrors.test.ts Tested: Live Windows dev run -- Codex sessions complete tool calls without the OS file-association popup; engine-status probes return installed:true once cold-start storms warm up. Not-tested: macOS/Linux behavior on this branch -- only Windows-gated code paths changed, but POSIX hasn't been re-exercised end-to-end. Not-tested: A Windows host without Git for Windows installed -- the .cmd's stderr fallback path is logically correct but hasn't been observed in the wild.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/hl/stock/browser-harness-js/sdk/browser-harness-js.cmd">
<violation number="1" location="app/src/main/hl/stock/browser-harness-js/sdk/browser-harness-js.cmd:24">
P1: `%errorlevel%` is expanded inside a block, which can return a stale code and hide `bash.exe` failures. Use `exit /b` (without an argument) to propagate the current status reliably.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
`exit /b %errorlevel%` inside the parenthesized blocks expanded at parse time, so a failing bash.exe was reported back to Codex as exit 0 -- the agent saw success even when the harness script failed. `exit /b` without an argument propagates the *current* ERRORLEVEL, which is what we want. Same fix applied to both branches (BROWSER_HARNESS_JS_BASH override and the Git for Windows search loop). Tested: npx vitest run tests/unit/hl/harnessBootstrap.test.ts Not-tested: live bash exit-code propagation -- verified by inspection against cmd.exe documentation; the previous .cmd was already in PR.
Collaborator
Author
|
@cubic review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--remote-debugging-porton the default profile, so neither copy-then-launch nor launch-against-original returns plaintext cookies. Until v20 ABE decryption is built natively, hide the surface.pollInstalledStatuswatchesdetectClaudeCode/detectCodexuntil the user runs it manually. Backgroundcmd.exeinvocations ofnpm install -g …have been unreliable on real machines; this is more predictable..cmdwins the PATHEXT tiebreaker over.ps1(cmd.exe avoids thespawn powershell.exe ENOENTand PowerShell cold-start hits we observed inside Electron's main process), and.ps1-only packages route through the absolute%SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exepath so the fallback no longer depends onenv.Pathlookup.browser-harness-js.cmdalongside the bash CLI so Codex's RustCreateProcessWfinds a PATHEXT-resolvable entry that delegates to Git Bash (which Claude Code already uses directly). Stops the OS "How do you want to open this file?" dialog on every browser-harness call from Codex tasks. Claude Code is unaffected — it still finds the bare bash script via shebang.macOS and Linux flows are byte-identical to before. The Windows-only branches are gated on
window.onboardingAPI.platform === 'win32'(onboarding) andwindow.electronAPI.shell.platform === 'win32'(hub).Test plan
npm run typecheckcleannpm run lint0 errorsnpx vitest run tests/unit/hl/harnessBootstrap.test.ts tests/unit/hl/codexStdinWindows.test.ts tests/unit/pathEnrich.test.ts tests/unit/identity/codexLogin.test.ts tests/unit/onboarding/OnboardingApp.spec.tsx tests/unit/renderer/ipcErrors.test.tsall passinstalled: trueonce cold-start storms warm upSummary by cubic
Stabilizes the Windows Codex flow by hiding cookie sync on Windows, switching engine installs to a copy-to-clipboard + poll flow, preferring
.cmdshims, and adding a Windows launcher forbrowser-harness-jswith proper exit-code handling.@anthropic-ai/claude-codeand@openai/codex..cmdover.ps1; route.ps1via absolute%SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exeto avoid ENOENT and cold starts.browser-harness-js.cmdthat invokes Git Bash; avoids the OS "Open with…" dialog, shows a clear error if Git is missing, and propagates bash exit codes correctly.profilestep on Windows so users don’t land on a hidden cookie-sync page; stepper updates accordingly.Written for commit 4f3a9ec. Summary will update on new commits.