Repository navigation
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens snapshot capture and Desktop sidecar lifecycle handling by avoiding stale Git index locks during snapshot operations, ensuring early child-process failures don’t get masked by late stdin pipe errors, and preventing Desktop from keeping an unexpectedly-terminated sidecar marked as the active server.
Changes:
- Snapshot capture now uses a scoped temporary Git index (
GIT_INDEX_FILE) and snapshot diffs/patches compare tree hashes rather than relying on--cached. - Child-process spawning now settles/interrupts configured stdin before exposing exit, preserving original non-zero exit status/diagnostics over late pipe errors; tests added.
- Desktop sidecar lifecycle now tracks “expected” vs “unexpected” exits and clears the current server reference only when the current sidecar terminates unexpectedly; tests added.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/opencode/test/snapshot/snapshot.test.ts | Adds regression coverage for stale shared index.lock not blocking snapshot capture. |
| packages/opencode/src/snapshot/index.ts | Implements temp-index snapshot capture and updates patch/diff logic to compare tree hashes. |
| packages/desktop/src/main/sidecar-lifecycle.ts | Introduces a small lifecycle helper to classify expected vs unexpected sidecar exits. |
| packages/desktop/src/main/sidecar-lifecycle.test.ts | Adds unit coverage for lifecycle classification and “current sidecar” checks. |
| packages/desktop/src/main/server.ts | Exposes a sidecar exit settlement promise and marks stop-initiated exits as expected. |
| packages/desktop/src/main/index.ts | Clears the active server reference only on unexpected exit of the current sidecar. |
| packages/core/test/process/process.test.ts | Adds coverage ensuring early git failure isn’t replaced by a secondary stdin pipe error. |
| packages/core/test/effect/cross-spawn-spawner.test.ts | Adds coverage that stdin is settled before exit is observed. |
| packages/core/src/cross-spawn-spawner.ts | Changes exitCode behavior to settle stdin and preserve primary exit failures over pipe errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const add = Effect.fnUntraced(function* (env?: Record<string, string>) { | ||
| yield* sync() | ||
| const [diff, other] = yield* Effect.all( | ||
| [ |
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
TLDR;
Problem
OpenCode snapshots use Git with a NUL-delimited path list written to child-process stdin. If the snapshot repository contains a stale
index.lock, Git exits before consuming that input. On Windows, the outstanding overlapped pipe write can then complete aswrite EOF; if stdin lifecycle finishes after process completion, that late socket error can terminate the Desktop sidecar instead of preserving Git's real exit status and lock diagnostic.The shared snapshot index also means the stale lock continues blocking later captures. This change isolates each capture in a scoped alternate index, persists its baseline as an immutable Git tree ref, and settles child stdin before exposing process completion.
Headless repro
Run with Node 24 and Git for Windows. This uses the same
git add --pathspec-from-file=- --pathspec-file-nulshape as snapshot capture:Before this change, the process terminates with the same failure observed in Desktop diagnostics:
Changelog
Fixed
Improved