Skip to content

Conversation

@caiqinghua
Copy link
Contributor

  1. there is no opencode
  2. pnpm dev error
  3. install opencode
  4. always error

@benjaminshafii
Copy link
Member

Tried to repro locally from a fresh worktree. With PATH stripped of opencode, fails before build.rs runs because node_modules/tauri aren’t installed in this environment, so I couldn’t fully validate runtime behavior yet.

Reviewing the change set, I’d like a couple adjustments before merge:

  1. Avoid mutating in-place. The prepare/restore pair is brittle if fails or is interrupted, leaving a dirty config. Prefer a release-only config (ex: passed via ) or a single wrapper script that runs build and restores in a so restore is guaranteed.

  2. If we keep externalBin out of the default config for dev, keep the release config as the single source of truth (no backups in-tree). That avoids committing transient diffs or leaving a stale file on failure.

  3. The change to PATH in dev looks right; for release, the hard error is ok, but please keep the error message user-facing (“install opencode or set OPENCODE_BIN_PATH”) and fail only in release builds.

Let me know if you want me to wire up a clean release config + build wrapper; I can take it from here.

@benjaminshafii
Copy link
Member

Follow-up (previous comment got garbled by shell quoting).

Tried to repro locally from a fresh worktree. With PATH stripped of opencode, pnpm -C packages/desktop dev fails before build.rs runs because node_modules/tauri aren't installed in this environment, so I couldn't fully validate runtime behavior yet.

Reviewing the change set, I'd like a couple adjustments before merge:

  1. Avoid mutating tauri.conf.json in-place. The prepare/restore pair is brittle if tauri build fails or is interrupted, leaving a dirty config. Prefer a release-only config (ex: tauri.conf.release.json passed via tauri build --config ...) or a single wrapper script that runs build and restores in a try/finally so restore is guaranteed.

  2. If we keep externalBin out of the default config for dev, keep the release config as the single source of truth (no backups in-tree). That avoids committing transient diffs or leaving a stale .backup file on failure.

  3. The engineSource change to PATH in dev looks right; for release, the hard error is ok, but please keep the error message user-facing (install opencode or set OPENCODE_BIN_PATH) and fail only in release builds.

Let me know if you want me to wire up a clean release config + build wrapper; I can take it from here.

@caiqinghua
Copy link
Contributor Author

@benjaminshafii Yes, keeping tauri.conf.json immutable is better design. We should only remove create_debug_stub() to avoid confusion.

@benjaminshafii
Copy link
Member

Repro update: on this PR branch, with PATH excluding opencode but including cargo (~/.cargo/bin), pnpm -C packages/desktop dev builds and launches successfully (no sidecar warning anymore). The process just keeps running as expected; my command timed out after the app launched. So dev no longer hard-requires opencode, which is good.

I still think we should avoid in-place tauri.conf.json edits for release builds (same notes as before). If you want me to wire up a release-only config or safer wrapper, I can do it.

@caiqinghua
Copy link
Contributor Author

Yes, agree. I have removed tauri.conf.json edits.

@caiqinghua
Copy link
Contributor Author

@benjaminshafii ben, is there any other problem?

@benjaminshafii
Copy link
Member

Automated message: removing the debug stub is the right direction, but this now hard‑fails build.rs whenever OpenCode is missing—even in dev. That blocks local dev and conflicts with the “graceful degradation” goal.

Suggestion: only hard error in release builds; in dev log a warning and continue with PATH mode. Also this PR is currently CONFLICTING with dev, so please rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants