Skip to content

feat(providers): wire docker dispatch and image cache UX#1352

Draft
mvanhorn wants to merge 6 commits intostablyai:mainfrom
mvanhorn:feat/docker-dispatch-wiring
Draft

feat(providers): wire docker dispatch and image cache UX#1352
mvanhorn wants to merge 6 commits intostablyai:mainfrom
mvanhorn:feat/docker-dispatch-wiring

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

@mvanhorn mvanhorn commented May 2, 2026 •
edited
Loading

Summary

Closes the docker isolation series. Wires the Docker provider triple from #1349 into the inline dispatch in src/main/ipc/pty.ts and src/main/ipc/filesystem.ts. When a worktree carries isolation: 'docker' (set by the toggle in #1350), pty operations route through the Docker provider triple instead of local. Existing host and SSH paths are unchanged.

This PR also addresses the three residuals that PR #1349 disclosed in its AI Review Report:

  • node-pty wrapping for docker exec so interactive programs see a real terminal in packaged Electron builds
  • Search flag honoring (regex, wholeWord, include / exclude patterns) in DockerFilesystemProvider.search via rg-in-container
  • Binary blob detection in DockerGitProvider.getDiff and getBranchDiff

Adds a "Docker Images" settings pane plus IPC for docker:list-cached-images and docker:prune-image. Prune only touches images carrying the orca label.

This is PR 3 of 3:

Screenshots

UI changes: a new "Docker Images" pane in Settings showing cached images (id, size, last-used, prune button). The toggle UI from #1350 is unchanged here.

The terminal / file-explorer / source-control side has no new visual elements; the change is routing under the hood.

Screenshots will land before merge once the toggle in #1350 lands so the demo can show: enable toggle on a worktree, agent runs in a container, file edit syncs, then prune the image.

Testing

  • pnpm lint
  • pnpm typecheck
  • pnpm test (full suite, includes 165 tests across 18 docker-related files)
  • pnpm build
  • Coverage added: container registry idempotency, worktree-isolation lookup, dispatch routing in pty.ts and filesystem.ts (host and SSH paths verified unchanged), node-pty wrapping flag, search flag honoring, binary detection, image listing, prune label-guard, shell-wrapped command spawn, path-traversal guard.

AI Review Report

A code review pass was run before submission. Risks checked and findings:

  • Dispatch correctness: existing local + SSH branches in pty.ts and filesystem.ts are reached by the same call shapes as before. Tests assert no regressions on those paths.
  • Container lifecycle: docker-container-registry.ts is idempotent on getOrCreateContainer (multiple panes for one worktree share one container) and terminates cleanly when the worktree closes.
  • node-pty wrapping (PR 1 residual): tty: true paths now use node-pty.spawn('docker', ...) instead of child_process.spawn with piped stdio. Matches LocalPtyProvider.
  • Search flags (PR 1 residual): DockerFilesystemProvider.search now shells rg into the container with the same flag set the local search pipeline uses.
  • Binary detection (PR 1 residual): getDiff and getBranchDiff use git cat-file -t plus a 8KB null-byte sniff, mirroring isBinaryBuffer in src/main/ipc/filesystem.ts.
  • Cross-platform: paths via path.posix.join for in-container, path.join for host. Windows host->container path translation goes through the helper from feat(providers): docker provider foundation (pty / filesystem / git) #1349 (with cross-platform tests).
  • File naming: concrete domain names. No helpers / utils.

Three findings surfaced. Two fixed, one disclosed:

  • Fixed: pty:spawn with args.command (e.g., bash setup.sh, claude "prompt") now wraps through /bin/sh -c <command> instead of treating the command string as the docker exec executable. Setup runners and agent launches start correctly inside the container.
  • Fixed: path-traversal guard in docker-worktree-paths.ts only rejects exact .., ../, ..\\, or absolute paths. Filenames legitimately starting with .. (e.g., ..cache/file) map into the container as expected.
  • Disclosed (not fixed): Filesystem and Git IPC handlers route to the Docker provider only when the renderer call passes args.worktreeId. Today, the file explorer, editor, search, and source-control IPC signatures don't all carry worktreeId — they pass worktreePath and optional connectionId. So Docker filesystem / git routing currently activates only on the subset of handlers that already accept worktreeId. PTY routing works end-to-end. A follow-up PR (3b) will plumb worktreeId through the preload API surfaces and renderer call sites so the routing applies to all worktree-scoped operations. This is structural rather than a bug — the wiring is correct, the renderer just doesn't tell main which worktree it's acting on for non-PTY operations yet.

Security Audit

Container processes run with the host user's uid/gid. No host paths leak in beyond the bind-mounted worktree at /workspace. Path translation (host->container) goes through the dedicated helper with vitest coverage including the parent-relative guard.

docker:prune-image only acts on images with the orca label and a tag matching the convention orca:<repo>:<dockerfile-hash>. Reject all other ids; confirmed in tests.

docker:list-cached-images reads docker image ls with the label=orca filter so user-managed images are never enumerated.

pty:spawn for docker-isolated worktrees: command strings are wrapped through /bin/sh -c. The shell argv is built as ['/bin/sh', '-c', command] (separate array entries, no shell-string interpolation in the host process). Container process supervision is handled by the node-pty wrapper around docker exec, same lifecycle semantics as the local PTY.

No new outbound network surface from Orca itself. Whatever the agent does inside its container is governed by the user's existing agent trust path.

Notes

This PR builds on #1349 and #1350. Because neither has merged yet, this PR's diff against main is cumulative across all three PRs in the series. After #1349 and #1350 merge, this branch will rebase to its true delta.

Series:

Recommended merge order: 1, 2, 3 sequentially.

The disclosed worktreeId plumbing limitation in the AI Review Report above is a real gap. If the maintainers want a clean end-to-end demo before merging this series, splitting it into PR 3a (this PR, wiring + fixes) and PR 3b (worktreeId plumbing through preload + renderers) keeps each surface narrow. Open to direction.

X: @mvanhorn

AI was used for assistance.

mvanhorn added 6 commits May 1, 2026 23:36
@mvanhorn
feat(providers): docker provider foundation (pty / filesystem / git)
2071bb4 
Adds Docker as a fourth provider triple alongside local and SSH.
Pure plumbing: pty / filesystem / git providers + docker engine
detection + image build + container lifecycle + bind-mount path
resolution. No user-facing UI yet, no dispatch wiring.

Mirrors the SSH provider triple's API. Cross-platform: macOS
(Docker Desktop / Colima), Linux (Docker Engine, rootless), Windows
(Docker Desktop WSL2 named pipe).

Tests cover happy and error paths through a docker-engine-fake.ts
recorder, so no real Docker daemon is required.

Foundation PR for the docker isolation series. Follow-ups will add
the user-facing toggle and dispatch wiring.
@mvanhorn
fix(docker): address self-review findings
387576f 
- Allocate TTY for Docker PTY sessions via tty flag on spawnExec, so
  isatty() returns true for shells, vim, and ncurses programs.
- Return originalContent + modifiedContent for working-tree diffs
  instead of returning a unified patch as modifiedContent.
- Return blob contents for branch diffs and honor options.oldPath for
  renames, matching the local provider's GitDiffResult shape.
- Preserve the new path as path and the old path as oldPath when
  parsing R100 rename entries (was reversed).
- Surface git porcelain v2 u records as unresolved conflict entries
  so docker worktrees in conflict show their conflicted files.
- Resolve repo default branch via origin/HEAD before browseFile,
  instead of hardcoding 'main' for remote file URLs.
@mvanhorn
feat(worktree): docker isolation toggle and image build flow
63c7dd5 
Adds an opt-in "Isolate" toggle per worktree row. When enabled, Orca
builds (or reuses cached) a Docker image via the foundation in the
docker provider series and persists the choice. Default off.

Repo-level default toggle in Repository Settings: "Isolate worktrees
from this repo by default."

Implementation:
- 3 new IPC handlers in src/main/ipc/docker.ts (engine-status,
  build-image, set-worktree-isolation) with build-progress events
- IsolateToggleButton.tsx covers off / building / on / disabled-no-docker
  states with vitest coverage
- RepositoryPane gains the per-repo defaultIsolation toggle
- Worktree shape extends with isolation field; defaultIsolation flows
  through worktree-remote so new worktrees inherit the repo default
- vitest.config.ts updated so .test.tsx files are discovered for the
  new component tests
@mvanhorn
fix(docker): disable isolation toggle for SSH-mounted repos
775641e 
Building a local Docker image against an SSH repo's remote path would
fail or target the wrong machine. Add a guard in both the main process
and the renderer:

- buildImage IPC returns an explicit error when repo.connectionId is set
- IsolateToggleButton disables itself for SSH repos with a clear tooltip

Local-only Docker isolation is the v1 surface. SSH-repo container
isolation is a separate problem (would require routing the build
through the SSH provider) and is out of scope for this series.
@mvanhorn
feat(providers): wire docker dispatch and image cache UX
a338623 
Closes the docker isolation series. When a worktree has
isolation: 'docker', pty / filesystem / git operations route through
the Docker provider triple (built in stablyai#1349, surfaced in stablyai#1350) instead
of the local provider. Existing host and SSH paths unchanged.

Implementation:
- worktree-isolation-lookup.ts: small Store-backed helper
- docker-container-registry.ts: per-worktree container cache. Spawn on
  first attach, terminate on worktree close
- pty.ts: getProvider extended to route by isolation
- filesystem.ts: third dispatch branch for docker-isolated worktrees
  alongside the existing local + SSH branches; includes git ops

Residuals from stablyai#1349 fixed in this PR:
- node-pty wrapping for docker exec (so interactive programs see a
  real terminal in packaged Electron builds)
- search flag honoring (useRegex, wholeWord, include/exclude patterns)
  in DockerFilesystemProvider.search via rg-in-container
- binary blob detection in DockerGitProvider.getDiff and getBranchDiff

Adds an "Docker Images" settings pane plus IPC handlers for
docker:list-cached-images and docker:prune-image. Prune only touches
images carrying the orca label.
@mvanhorn
fix(docker): shell-wrap pty commands and tighten path traversal check
f8d8b64 
Two findings from the dispatch self-review:

- DockerPtyProvider.spawn now wraps opts.command in /bin/sh -c
  instead of treating the whole command string as the docker exec
  executable. So pty:spawn calls like `bash setup.sh` or
  `claude "prompt"` start a shell inside the container and run
  the command, matching LocalPtyProvider semantics.
- docker-worktree-paths only rejects true parent-relative paths
  (exactly `..`, `../`, `..\`, or an absolute path) instead of any
  string that happens to start with `..`. Filenames like `..cache`
  or `..tmp` now map into the container as expected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mvanhorn