Skip to content

fix(desktop): stabilize snapshot sidecar lifecycle#31283

Open
Hona wants to merge 4 commits into
anomalyco:devfrom
Hona:fix/snapshot-process-lifecycle
Open

fix(desktop): stabilize snapshot sidecar lifecycle#31283
Hona wants to merge 4 commits into
anomalyco:devfrom
Hona:fix/snapshot-process-lifecycle

Conversation

@Hona
Copy link
Copy Markdown
Member

@Hona Hona commented Jun 7, 2026

TLDR;

  • Snapshot capture no longer gets stuck behind a stale internal Git index lock.
  • Early Git failures no longer leave a late pipe error that can terminate the local Desktop server.
  • Desktop no longer keeps a terminated local server marked as active.

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 as write 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-nul shape as snapshot capture:

// repro.mjs
import { spawn, spawnSync } from "node:child_process"
import { mkdirSync, rmSync, writeFileSync } from "node:fs"
import { join } from "node:path"

const root = join(import.meta.dirname, "snapshot-eof-repro")
const worktree = join(root, "worktree")
const gitdir = join(root, "snapshot.git")
rmSync(root, { recursive: true, force: true })
mkdirSync(worktree, { recursive: true })
spawnSync("git", ["init", "--bare", gitdir])
writeFileSync(join(gitdir, "index.lock"), "")

const input = Buffer.alloc(4 * 1024 * 1024, 0x61)
for (let i = 255; i < input.length; i += 256) input[i] = 0

const child = spawn("git", [
  "--git-dir", gitdir,
  "--work-tree", worktree,
  "add", "--all",
  "--pathspec-from-file=-",
  "--pathspec-file-nul",
], { stdio: ["overlapped", "pipe", "pipe"], windowsHide: true })

child.stdin.end(input)
node repro.mjs

Before this change, the process terminates with the same failure observed in Desktop diagnostics:

Error: write EOF
    at WriteWrap.onWriteComplete [as oncomplete]
Emitted 'error' event on Socket instance
code: 'EOF'
syscall: 'write'

Changelog

Fixed

  • OpenCode Desktop no longer loses snapshot capture when an old internal snapshot lock remains on disk.
  • Files already tracked by snapshots remain tracked after migration, including files that later grow beyond the new-file size limit.
  • The local Desktop server no longer terminates from a delayed input-pipe error after Git exits early.
  • Git failures retain their original exit status and diagnostic output instead of being replaced by a secondary pipe error.
  • Successful commands remain successful when they intentionally stop reading input before it is exhausted.
  • An unexpectedly terminated local server is no longer kept as the current Desktop server connection.
  • A previous local server process exiting after replacement no longer clears the newer active server.

Improved

  • Snapshot capture now uses a temporary scoped Git index and a Git-managed baseline ref while preserving existing snapshot retention, pruning, serialization, restore, and revert behavior.
  • Intentional Desktop shutdown is distinguished from an unexpected local server exit in diagnostics.

@Hona Hona requested a review from adamdotdevin as a code owner June 7, 2026 23:34
Copilot AI review requested due to automatic review settings June 7, 2026 23:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +196 to 199
const add = Effect.fnUntraced(function* (env?: Record<string, string>) {
yield* sync()
const [diff, other] = yield* Effect.all(
[
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