fix(desktop): avoid incompatible startup shells - #674
Merged
Conversation
Restrict POSIX desktop launch scripts to Bash and Zsh instead of passing them to any executable named by SHELL. Fall back to the system Zsh on macOS and Bash on Linux so Nushell and Fish users no longer see a blank window while the backend startup script fails to parse. Keep compatible configured shell paths and their existing login environment behavior intact. Apply the same selection policy to Tauri and Electron so both desktop hosts launch consistently. Add focused Rust and Node tests for incompatible-shell fallback, compatible-shell preservation, and platform defaults. Validate the complete Tauri and Electron native suites, desktop typechecks, formatting, and the production Electron build.
Contributor
Author
pascalandr
commented
Sep 5, 2026
pascalandr
left a comment
pascalandr
left a comment
Contributor
Author
There was a problem hiding this comment.
Gatekeeper review of d7e7fba6
No blocking code findings were found in the Electron/Tauri shell-selection parity change.
Both hosts now accept only Bash or Zsh for the POSIX launch script, trim configured paths, and fall back deterministically to /bin/zsh on macOS or /bin/bash elsewhere instead of sending POSIX syntax to Fish/Nushell.
Validation
- Electron shell tests: 2/2 passed.
- Tauri shell-selection tests: 2/2 passed.
- Rust formatting and
git diff --checkpassed.
Gate status
Code-approved, CI-blocked. The current Linux failure is the inherited automation-registry test fixed by #671, not a defect in this branch. Rerun the complete PR matrix after #671 lands before merging.
pascalandr
added a commit
that referenced
this pull request
Sep 6, 2026
Update #672 with dev at 81aa4d2, including the integrated #671 registry isolation, #674 POSIX shell selection and #677 window persistence fixes. This brings in the previously missing Linux CI prerequisite without duplicating its patch. Resolve the sole conflict in pr-build.yml by retaining both the OpenCode compatibility matrix and macOS ARM64 Tauri test job, and requiring both before packaging. Every non-build job is structurally identical to its originating parent and every parent build dependency is preserved. No application code needed manual conflict resolution. Validation after integration: 391 server tests passed with 2 skipped, all 192 Electron tests passed, and server/UI/Electron typechecks passed. YAML parsing, exact parent-job comparisons and git diff --check pass. The complete remote matrix must rerun on the combined branch before merging the PR.
pascalandr
added a commit
that referenced
this pull request
Sep 6, 2026
## Summary Closes #669. CodeNomad V2 requires the official OpenCode V2 shared-service lifecycle. A legacy binary must not be accepted merely because `--version` succeeds, nor should opening a folder display a raw legacy CLI help dump. This is an actionable compatibility fix, **not V1 runtime support or automatic installation**: install/select `opencode2` to open the workspace. - Validate the selected executable using only `--version` and `service --help`; neither probe starts or changes the daemon. - Require positive service-help evidence for `start`, `status`, and `get`, without pinning a release number or rejecting custom version labels. - Recognize legacy root help, including ANSI/CRLF, stdout/stderr, and exit-zero wrappers. Preserve the localized V2 diagnosis for previously saved incompatible paths during the actual host/WSL lifecycle. - Run both validation probes asynchronously with a 5-second subprocess timeout and 64 KiB output limit, leaving the shared backend responsive while a CLI is slow. - Treat saved versions as display metadata only: explicit add/browse always validates the path again. - Match the compatibility error code precisely, including its JSON HTTP envelope, so unrelated missing-file/configuration errors are not hidden by a matching filename. - Reuse the existing translated message in all ten locales. No daemon ownership, V1 fallback, package update, or native-shell-selection change. ## Follow-up verification of the original PR The original head `226e2d7b` already fixed the exact reported failure with a real V1 binary. Independent rechecking found and corrected adjacent gaps in this same PR: | Check | Original PR | Completed PR | | --- | --- | --- | | Real OpenCode 1.18.25 rejected; real V2 accepted | PASS | PASS | | Empty/unrelated successful help rejected | FAIL | PASS | | Legacy help with exit zero localized at launch | FAIL | PASS | | Other backend work proceeds during slow probes | FAIL | PASS | | Explicit add rechecks a persisted V1 version | FAIL | PASS | | Unrelated diagnostic mentioning `opencode_v2_required` retained | FAIL | PASS | The new regression tests were run against the old production code first: three assertions failed (false capability acceptance, exit-zero help, unrelated-error masking), then passed after the fix. A separate source-extraction check executes the unchanged published probe and component validation body: two 250 ms subprocess delays block the old probe for about 638 ms, versus a Promise returned in about 5 ms after the fix; a cached V1 version previously produced zero validation calls and now produces one and rejects the path. ## Validation of the compatibility fix (`903d9159`) - Full server suite on Windows: **391 passed, 2 skipped**. - Electron native suite: **189 passed**. - Focused compatibility/lifecycle/updater/HTTP/UI diagnostic suite: **79 passed** on Windows. - Server, UI and Electron typechecks passed; server and UI production builds passed. - `git diff --check` and workflow YAML validation passed. - Real CLI checks: V1 **1.18.25** refused with `opencode_v2_required`; V2 beta builds **19151/19192** accepted; authenticated discovery of the existing shared daemon succeeds. No service stop/restart or settings mutation was performed. - Actual subprocess fixtures exercise spaced/apostrophe/Unicode paths, stdout/stderr and exit codes. HTTP integration proves that validation neither mutates preferences nor blocks another request. - Existing regressions retain stopped/running daemon discovery, startup environment isolation, password-error redaction, deadlines, WSL command forwarding, authenticated health, shared-service ownership and updater behavior. - Independent focused CI matrix passed on **Linux and macOS 26 ARM64** (77 passed, 2 Windows-only skips each) and **Windows** (79 passed), in run `34036886140`. ## Update with current `dev` / conflict resolution Updated this branch with `dev` at `81aa4d24`, which now includes **#671, #674 and #677**. The only textual conflict was in `.github/workflows/pr-build.yml`, where #672 and #677 added CI jobs at the same location. Both the OpenCode compatibility matrix and the macOS ARM64 Tauri tests are retained, with both required by the package-build gate. Parsed workflow comparisons verify that every non-build job is identical to its originating parent and no build dependency was dropped. The previously blocking Linux automation-registry fix **#671 is now included through `dev`**, rather than duplicated. The full CI matrix is rerunning on this combined state; require that result before merge. Local checks after resolving the conflict: **391 server tests passed, 2 skipped; 192 Electron tests passed; server/UI/Electron typechecks passed; workflow parent-preservation assertions and `git diff --check` passed**. The conflict resolution itself changes no application code. Automated CLI/HTTP tests are not an interactive Electron/macOS ARM64 smoke test on the reporter's machine, and cannot establish the absolute absence of every possible side effect. Maintenance: files already touched by the original PR remain oversized: `packages/server/src/api-types.ts` (536 lines) and `packages/ui/src/App.tsx` (855 lines). This follow-up does not grow those files.
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
2 participants
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
$SHELLvalues to/bin/zshon macOS and/bin/bashon LinuxValidation
cargo fmt --checkcargo test --locked -- --test-threads=1— 141 passednpm run test:native --workspace @neuralnomads/codenomad-electron-app— all passednpm run typechecknpm run build --workspace @neuralnomads/codenomad-electron-app/opt/homebrew/bin/nuselects/bin/zshfor a macOS launchA packaged macOS runtime smoke test is not available from the Windows development host; the platform selection is isolated and tested without relying on the host OS.
Closes #670