Skip to content

effect(snapshot): migrate to AppProcess.run#27189

Merged
kitlangton merged 1 commit into
devfrom
effect/phase2-snapshot-appprocess
May 13, 2026
Merged

effect(snapshot): migrate to AppProcess.run#27189
kitlangton merged 1 commit into
devfrom
effect/phase2-snapshot-appprocess

Conversation

@kitlangton

Copy link
Copy Markdown
Contributor

Phase 2 sweep: migrate src/snapshot/index.ts from raw ChildProcessSpawner to AppProcess.

Stacked on #27178.

Migrated call sites

All non-stdin git invocations now go through appProcess.run(ChildProcess.make("git", ...)) via a thin git helper that translates RunResult -> the existing GitResult shape. Uses interpretExitCode: () => "success" so non-zero exits flow through (preserving the prior behavior where the helper swallowed errors via Effect.catch and returned { code: 1, ... }).

Call sites flowing through the new git helper:

  • excludes()git rev-parse --git-path info/exclude
  • add()git diff-files --name-only -z, git ls-files --others --exclude-standard -z
  • cleanup()git gc --prune=<duration>
  • track()git init, git config core.* x4, git write-tree
  • patch()git diff --cached --no-ext-diff --name-only <hash>
  • restore()git read-tree <snapshot>, git checkout-index -a -f
  • revert()git checkout, git ls-tree (single + batched paths)
  • diff()git diff --cached --no-ext-diff <hash>
  • diffFull()git show <ref>:<file> (per-file fallback), git diff --name-status, git diff --numstat

Stayed on raw spawn (appProcess.spawn)

Four sites feed bytes into git's stdin via Stream, and AppProcess.run does not yet expose a stdin-Stream API. They now use appProcess.spawn(...) (inherited platform method) — same shape as before, no behavior change:

  • ignore()git check-ignore --stdin -z (NUL-delimited path list)
  • drop()git rm --cached --pathspec-from-file=- --pathspec-file-nul
  • stage()git add --all --sparse --pathspec-from-file=- --pathspec-file-nul
  • diffFull() load()git cat-file --batch (newline-delimited ref list)

These are isolated in a gitWithStdin helper alongside git to make the distinction explicit at the call site.

AppProcess API feedback

A runStdin(command, { stdin: Stream<Uint8Array> }) helper (or a stdin field on RunOptions) would let these last four sites migrate. Filing as future work — out of scope for this PR.

LOC delta

+45 / -14 in a single file (src/snapshot/index.ts).

Verification

  • bun run test test/snapshot/ -> 52 pass, 1 skip, 0 fail
  • bun run test test/session/snapshot-tool-race.test.ts -> 1 pass, 0 fail
  • bun run typecheck -> 0 errors

@kitlangton kitlangton force-pushed the effect/app-process-service branch from ebc8050 to bf596d5 Compare May 13, 2026 01:15
@kitlangton kitlangton force-pushed the effect/phase2-snapshot-appprocess branch from 802609d to a0f3716 Compare May 13, 2026 01:20
Base automatically changed from effect/app-process-service to dev May 13, 2026 01:24
Route non-stdin git calls through AppProcess.run. Keep stdin-feed
calls (check-ignore --stdin, rm --pathspec-from-file=-, add
--pathspec-from-file=-, cat-file --batch) on raw spawn — AppProcess.run
does not yet accept a stdin Stream.
@kitlangton kitlangton force-pushed the effect/phase2-snapshot-appprocess branch from a0f3716 to 651c915 Compare May 13, 2026 14:32
@kitlangton kitlangton marked this pull request as ready for review May 13, 2026 14:32
@kitlangton kitlangton merged commit 766318a into dev May 13, 2026
10 checks passed
@kitlangton kitlangton deleted the effect/phase2-snapshot-appprocess branch May 13, 2026 14:46
AIALRA-0 pushed a commit to AIALRA-0/opencode-turn-engine that referenced this pull request Jun 10, 2026
AIALRA-0 pushed a commit to AIALRA-0/opencode-turn-engine that referenced this pull request Jun 10, 2026
avion23 pushed a commit to avion23/opencode that referenced this pull request Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant