From cdb443b4e2e477d1e86765a94983d7f449f2680c Mon Sep 17 00:00:00 2001 From: Clint Berry Date: Tue, 28 Jul 2026 21:31:55 +0000 Subject: [PATCH 1/4] feat(workspace): cache devcontainer images with agent tooling baked in Sessions rebuilt each repo's devcontainer from scratch and then installed Pi, pi-subagents and the ask-user extension over `devpod ssh`, so every cold start paid a full image build plus several install round-trips. DEUCE_PREBUILD_REPOSITORY (empty by default, which keeps the existing behaviour byte-for-byte) turns on a two-stage cache: devpod build --repository R --skip-push -> R:devpod- docker build (Deuce layer: Pi + tools) -> R:deuce- devpod up --devcontainer-image R:deuce- is devpod's hash of the devcontainer definition, so the cache invalidates on a devcontainer change and survives ordinary code pushes. Later sessions skip the build and the over-ssh provisioning entirely. --devcontainer-image rather than --prebuild-repository: the latter only ever does a registry lookup, so it cannot consume a local tag. Going through the image override keeps the whole cache local to the Docker daemon, no registry required. Every failure degrades instead of breaking a session - an unparseable build tag or a failed bake falls back to the from-scratch path and the original over-ssh install. The baked layer resolves HOME from the passwd database per RUN, because Docker's USER directive changes the uid but not $HOME; without that Pi installs into /root where the session's remoteUser cannot reach it. Verified end-to-end (opt-in DEUCE_PREBUILD_E2E test): cache hit on the second call, devpod up starts with no build step, and pi 0.82.1 plus the ask-user extension are present in-container as the vscode remoteUser. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014orMLVuaREojZaJ6mzXvtv --- CLAUDE.md | 10 + server/internal/config/config.go | 32 ++ server/internal/config/config_test.go | 59 +++ server/internal/handler/sessions.go | 11 +- server/internal/handler/workspace.go | 26 +- server/internal/server/server.go | 6 + server/internal/workspace/manager.go | 56 ++- server/internal/workspace/prebuild.go | 377 ++++++++++++++++++ .../internal/workspace/prebuild_e2e_test.go | 108 +++++ server/internal/workspace/prebuild_test.go | 329 +++++++++++++++ 10 files changed, 996 insertions(+), 18 deletions(-) create mode 100644 server/internal/workspace/prebuild.go create mode 100644 server/internal/workspace/prebuild_e2e_test.go create mode 100644 server/internal/workspace/prebuild_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 177755e..3412b5c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,6 +57,7 @@ cd server && make migrate # Run migrations + seed data - **Auth**: Two modes selected by `DEUCE_AUTH_MODE`. `dev` (default) injects a fixed user ID from `DEUCE_USER_ID` — localhost-only, do not bind on a non-loopback interface. `forge-proxy` trusts `X-Forge-*` headers from the [forge-proxy](https://github.com/forgeutah/forge-proxy) reverse proxy (validates the shared secret in constant time, checks a single required role from CSV, auto-provisions users by `forge_user_id`). See `server/internal/auth/forge_proxy.go`. - **Agent runtime**: One persistent Pi (`pi --mode rpc`) process per session inside its DevPod container, driven over JSONL (`server/internal/agent/`). `@deuce` mentions are detected server-side (`server/internal/handler/messages.go`) and enqueue tasks on the session's serial queue; `GET/PUT /api/agent` reads/edits deuce's global system prompt. The agent's fixed UUID is `agent.DeuceAgentID` (mirrored by `DEUCE` in `src/lib/deuce.ts`); the nil UUID is the system-notice author sentinel. - **DevPod**: Workspace manager shells out to `devpod` CLI via os/exec (`server/internal/workspace/manager.go`) +- **Devcontainer prebuild cache** (opt-in via `DEUCE_PREBUILD_REPOSITORY`, `server/internal/workspace/prebuild.go`): `devpod build --repository R --skip-push` builds the repo's devcontainer once and tags it `R:devpod-`, where `` is devpod's hash of the devcontainer definition. A thin Deuce layer then bakes Pi, pi-subagents and the ask-user extension on top as `R:deuce-`, and `devpod up --devcontainer-image R:deuce-` starts sessions from it with no build and no over-ssh provisioning. Because the tag carries the definition hash, a devcontainer change invalidates the cache while ordinary code pushes reuse it. ### Adding a New API Endpoint @@ -90,6 +91,15 @@ GITHUB_TOKEN= # GitHub PAT for repo listing (optional) DEVPOD_BIN=devpod # DevPod binary path DEVPOD_PROVIDER= # DevPod provider (empty = default) +# Devcontainer prebuild cache. Empty (default) = off: every session runs a +# from-scratch `devpod up` and installs Pi over `devpod ssh`. When set to a +# Docker repository name (no tag — Deuce appends the devcontainer hash), each +# repo's devcontainer image is built once per devcontainer-definition hash, +# has Pi + pi-subagents + the ask-user extension baked on top, and later +# sessions start straight from that image with no build and no over-ssh +# install. Tags stay local to the Docker daemon; no registry is required. +DEUCE_PREBUILD_REPOSITORY= + # Agent backend. Pi (pi.dev) runs in --mode rpc inside each session's DevPod # container, driven over a persistent JSONL channel — one process per session. # PiProvider/PiModel select the Pi backend (v1 runs Claude models through Pi). diff --git a/server/internal/config/config.go b/server/internal/config/config.go index 4637421..0837bd5 100644 --- a/server/internal/config/config.go +++ b/server/internal/config/config.go @@ -42,6 +42,19 @@ type Config struct { DevPodProvider string `env:"DEVPOD_PROVIDER" envDefault:"docker"` AnthropicAPIKey string `env:"ANTHROPIC_API_KEY" envDefault:""` + // PrebuildRepository turns on the devcontainer prebuild cache. Empty + // (the default) keeps the original behaviour exactly: every session + // runs a from-scratch `devpod up` and installs Pi over `devpod ssh`. + // When set, each repo's devcontainer image is built once per + // devcontainer-definition hash, has Pi and the agent tooling baked on + // top, and later sessions start from that cached image. + // + // The value is a Docker repository name. It does not have to resolve + // to a registry — tags stay local to the Docker daemon, because the + // image is consumed via `devpod up --devcontainer-image` rather than + // `--prebuild-repository` (which only ever does a registry lookup). + PrebuildRepository string `env:"DEUCE_PREBUILD_REPOSITORY" envDefault:""` + // PiProvider/PiModel configure the Pi agent backend (the sole harness); // v1 runs Claude models through Pi. PiProvider string `env:"DEUCE_PI_PROVIDER" envDefault:"anthropic"` @@ -124,6 +137,19 @@ func (c *Config) ProxyRoleCheckEnabled() bool { return c.ProxyHeaderRoles != "" } +// prebuildRepoRE matches a Docker repository name without a tag: optional +// registry host (with optional port), then one or more lowercase path +// components. Deliberately strict — the value reaches `docker build -t` and +// `devpod up --devcontainer-image` argv, and a tag separator here would +// collide with the hash tag Deuce appends. +var prebuildRepoRE = regexp.MustCompile(`^[a-z0-9]+([._-][a-z0-9]+)*(:[0-9]+)?(/[a-z0-9]+([._-][a-z0-9]+)*)*$`) + +// PrebuildEnabled reports whether the devcontainer prebuild cache is turned +// on. When false, workspace creation keeps its original from-scratch path. +func (c *Config) PrebuildEnabled() bool { + return c.PrebuildRepository != "" +} + // Validate checks the config for self-consistency before the server binds. // In proxy mode it refuses to start when the optional-check env-var pairs // are asymmetric (a header without its value, or vice versa), when the @@ -145,6 +171,12 @@ func (c *Config) Validate() error { return errors.New("DEUCE_WS_ALLOWED_ORIGINS cannot contain '*' — wildcard origins re-open cross-site WebSocket hijacking") } + // Caught at startup rather than at first session create, where the + // failure would surface as a confusing mid-provisioning docker error. + if c.PrebuildRepository != "" && !prebuildRepoRE.MatchString(c.PrebuildRepository) { + return fmt.Errorf("DEUCE_PREBUILD_REPOSITORY=%q is not a valid Docker repository name (lowercase, no tag — Deuce appends the devcontainer hash as the tag)", c.PrebuildRepository) + } + if c.AuthMode == AuthModeProxy { return c.validateProxyMode(origins) } diff --git a/server/internal/config/config_test.go b/server/internal/config/config_test.go index 9223e0c..694cfeb 100644 --- a/server/internal/config/config_test.go +++ b/server/internal/config/config_test.go @@ -307,3 +307,62 @@ func TestWSAllowedOriginList_TrimsAndDropsEmpty(t *testing.T) { } } } + +func TestPrebuildEnabled(t *testing.T) { + cfg := &Config{} + if cfg.PrebuildEnabled() { + t.Error("prebuild should be off by default — an empty repository keeps the original from-scratch path") + } + cfg.PrebuildRepository = "deuce-prebuild" + if !cfg.PrebuildEnabled() { + t.Error("prebuild should be on when a repository is configured") + } +} + +func TestValidate_PrebuildRepositoryAccepted(t *testing.T) { + valid := []string{ + "", // off + "deuce-prebuild", // bare local name + "ghcr.io/forgeutah/deuce-prebuild", // registry path + "localhost:5000/deuce-prebuild", // registry with port + "deuce_prebuild", // underscore separator + } + for _, repo := range valid { + cfg := &Config{ + AuthMode: AuthModeDev, + WSAllowedOrigins: "localhost:4000", + PrebuildRepository: repo, + } + if err := cfg.Validate(); err != nil { + t.Errorf("PrebuildRepository=%q should validate: %v", repo, err) + } + } +} + +func TestValidate_PrebuildRepositoryRejected(t *testing.T) { + // A tag here would collide with the devcontainer hash Deuce appends, + // and the flag- and shell-shaped values must never reach docker argv. + invalid := []string{ + "deuce-prebuild:latest", + "Deuce-Prebuild", + "--build-arg=evil", + "repo with space", + "repo;rm -rf /", + "repo$(hostile)", + } + for _, repo := range invalid { + cfg := &Config{ + AuthMode: AuthModeDev, + WSAllowedOrigins: "localhost:4000", + PrebuildRepository: repo, + } + err := cfg.Validate() + if err == nil { + t.Errorf("PrebuildRepository=%q should be rejected", repo) + continue + } + if !strings.Contains(err.Error(), "DEUCE_PREBUILD_REPOSITORY") { + t.Errorf("error for %q should name the env var: %v", repo, err) + } + } +} diff --git a/server/internal/handler/sessions.go b/server/internal/handler/sessions.go index f956e2c..e0019cf 100644 --- a/server/internal/handler/sessions.go +++ b/server/internal/handler/sessions.go @@ -585,17 +585,18 @@ func (h *Handler) startWorkspace(sessionID uuid.UUID, workspaceID, repoURL strin h.hub.BroadcastToSession(sessionID.String(), msg, nil) } - err := h.workspaces.Create(ctx, workspaceID, repoURL, logFn) + prebuilt, err := h.workspaces.Create(ctx, workspaceID, repoURL, logFn) var newStatus string if err != nil { slog.Error("workspace creation failed", "sessionID", sessionID, "error", err) newStatus = "failed" } else { - // Install the agent harnesses (Claude fallback + Pi + ask-user - // extension) after workspace creation. Idempotent and non-fatal; the - // same helper runs on every start/rebuild so older containers migrate. - h.provisionAgentTools(ctx, workspaceID, logFn) + // Install the agent harnesses (Pi + subagents + ask-user extension) + // after workspace creation. Idempotent and non-fatal; the same helper + // runs on every start/rebuild so older containers migrate. Skipped + // when the workspace image already carries them. + h.provisionAgentToolsIfNeeded(ctx, workspaceID, prebuilt, logFn) newStatus = "ready" } diff --git a/server/internal/handler/workspace.go b/server/internal/handler/workspace.go index 1a980f6..d94ac87 100644 --- a/server/internal/handler/workspace.go +++ b/server/internal/handler/workspace.go @@ -34,6 +34,22 @@ func (h *Handler) provisionAgentTools(ctx context.Context, workspaceID string, l } } +// provisionAgentToolsIfNeeded runs the over-ssh install only when the +// container did not start from a Deuce-baked image. When the tooling is +// baked in, the installers would be a no-op that still costs several ssh +// round-trips on the session-open path — which is the cost the prebuild +// cache exists to remove. +func (h *Handler) provisionAgentToolsIfNeeded(ctx context.Context, workspaceID string, res workspace.PrebuildResult, logFn workspace.LogFunc) { + if res.ToolsBaked { + slog.Info("agent tooling baked into workspace image; skipping over-ssh provisioning", "workspace", workspaceID) + if logFn != nil { + logFn("Agent tooling already present in the workspace image") + } + return + } + h.provisionAgentTools(ctx, workspaceID, logFn) +} + // workspaceAction names the four lifecycle operations the user can trigger. // Each one maps to a transitional workspace_status that the handler writes // synchronously, then a devpod CLI call in a tracked background goroutine, @@ -223,12 +239,13 @@ func (h *Handler) runWorkspaceAction(sessionID uuid.UUID, workspaceID, repoURL s // devpod up is idempotent against existing workspaces: it resumes // a stopped container without rebuilding (verified — `--recreate` // is the explicit opt-in for a fresh container). - actErr = h.workspaces.Create(ctx, workspaceID, repoURL, logFn) + var prebuilt workspace.PrebuildResult + prebuilt, actErr = h.workspaces.Create(ctx, workspaceID, repoURL, logFn) if actErr == nil { // (Re)provision agent tooling on every start so containers created // before agent support — or where a prior install failed — pick up // Pi + the ask-user extension. The installers are idempotent. - h.provisionAgentTools(ctx, workspaceID, logFn) + h.provisionAgentToolsIfNeeded(ctx, workspaceID, prebuilt, logFn) newStatus = "ready" } else { newStatus = "failed" @@ -246,13 +263,14 @@ func (h *Handler) runWorkspaceAction(sessionID uuid.UUID, workspaceID, repoURL s // Delete + Create. Note: Manager.Delete uses CombinedOutput so its // output is not streamed via logFn — the user will see logs once // Create begins. Documented in the plan's Rebuild risk note. + var rebuilt workspace.PrebuildResult if delErr := h.workspaces.Delete(ctx, workspaceID); delErr != nil { actErr = fmt.Errorf("rebuild delete: %w", delErr) } else { - actErr = h.workspaces.Create(ctx, workspaceID, repoURL, logFn) + rebuilt, actErr = h.workspaces.Create(ctx, workspaceID, repoURL, logFn) } if actErr == nil { - h.provisionAgentTools(ctx, workspaceID, logFn) + h.provisionAgentToolsIfNeeded(ctx, workspaceID, rebuilt, logFn) newStatus = "ready" } else { newStatus = "failed" diff --git a/server/internal/server/server.go b/server/internal/server/server.go index 314584d..bde705b 100644 --- a/server/internal/server/server.go +++ b/server/internal/server/server.go @@ -121,6 +121,12 @@ func (s *Server) Router() http.Handler { } wm := workspace.NewManager(s.cfg.DevPodBin, s.cfg.DevPodProvider, s.cfg.GitHubToken) + // Only this manager creates workspaces; the sshproxy and reconciler + // managers in main.go never call Create, so they need no prebuild repo. + wm.SetPrebuildRepository(s.cfg.PrebuildRepository) + if s.cfg.PrebuildEnabled() { + slog.Info("devcontainer prebuild cache enabled", "repository", s.cfg.PrebuildRepository) + } if !wm.Available() { slog.Warn("devpod binary not found, workspace creation will be skipped") } else { diff --git a/server/internal/workspace/manager.go b/server/internal/workspace/manager.go index aa5aaab..bd696cb 100644 --- a/server/internal/workspace/manager.go +++ b/server/internal/workspace/manager.go @@ -66,6 +66,10 @@ type Manager struct { gitOnce sync.Once gitEnv []string + // prebuildRepo enables the devcontainer prebuild cache when non-empty + // (see prebuild.go). Empty keeps Create on its original path. + prebuildRepo string + // userMu guards userCache, which memoizes ContainerUser lookups. // VS Code Remote-SSH opens many channels per connection and each one // resolves the exec user, so an uncached `docker inspect` per channel @@ -180,12 +184,31 @@ func (m *Manager) EnsureDockerProvider(ctx context.Context) error { // Create starts a new DevPod workspace from a git repo URL. // Output is streamed line-by-line to logFn (if non-nil). // This blocks until the workspace is ready or fails. -func (m *Manager) Create(ctx context.Context, workspaceID, repoURL string, logFn LogFunc) error { - args := []string{"up", repoURL, "--id", workspaceID, "--ide", "none"} - if m.provider != "" { - args = append(args, "--provider", m.provider) +// +// When a prebuild repository is configured, the repo's devcontainer image is +// built once and cached with the agent tooling baked in, and the container +// starts from that image instead of building from scratch. The returned +// PrebuildResult reports whether the tooling was baked; ToolsBaked false +// means the caller must still run the over-ssh provisioning path. Any +// failure in the prebuild path degrades to the original behaviour rather +// than failing the session. +func (m *Manager) Create(ctx context.Context, workspaceID, repoURL string, logFn LogFunc) (PrebuildResult, error) { + var prebuilt PrebuildResult + if m.prebuildRepo != "" { + res, err := m.EnsurePrebuild(ctx, repoURL, logFn) + if err != nil { + slog.Warn("prebuild failed; falling back to from-scratch devcontainer build", + "id", workspaceID, "repo", repoURL, "error", err) + if logFn != nil { + logFn("WARNING: could not prepare a cached workspace image — building from scratch") + } + } else { + prebuilt = res + } } + args := devpodUpArgs(workspaceID, repoURL, m.provider, prebuilt.Image) + slog.Info("starting devpod workspace", "id", workspaceID, "repo", repoURL) cmd := exec.CommandContext(ctx, m.bin, args...) @@ -200,12 +223,12 @@ func (m *Manager) Create(ctx context.Context, workspaceID, repoURL string, logFn // Merge stderr into stdout so we capture everything stdout, err := cmd.StdoutPipe() if err != nil { - return fmt.Errorf("stdout pipe: %w", err) + return prebuilt, fmt.Errorf("stdout pipe: %w", err) } cmd.Stderr = cmd.Stdout if err := cmd.Start(); err != nil { - return fmt.Errorf("devpod start: %w", err) + return prebuilt, fmt.Errorf("devpod start: %w", err) } // Stream output line by line @@ -223,11 +246,26 @@ func (m *Manager) Create(ctx context.Context, workspaceID, repoURL string, logFn if logFn != nil { logFn(fmt.Sprintf("ERROR: devpod up failed: %v", err)) } - return fmt.Errorf("devpod up failed: %w", err) + return prebuilt, fmt.Errorf("devpod up failed: %w", err) } - slog.Info("devpod workspace ready", "id", workspaceID) - return nil + slog.Info("devpod workspace ready", "id", workspaceID, "toolsBaked", prebuilt.ToolsBaked) + return prebuilt, nil +} + +// devpodUpArgs assembles the `devpod up` argv. A non-empty image switches +// devpod off its build path entirely — it starts the container straight from +// that image, keeping the devcontainer.json's own metadata (features, +// remoteUser) because those travel on the image's labels. +func devpodUpArgs(workspaceID, repoURL, provider, image string) []string { + args := []string{"up", repoURL, "--id", workspaceID, "--ide", "none"} + if provider != "" { + args = append(args, "--provider", provider) + } + if image != "" { + args = append(args, "--devcontainer-image", image) + } + return args } // Stop halts a running workspace (can be resumed later). diff --git a/server/internal/workspace/prebuild.go b/server/internal/workspace/prebuild.go new file mode 100644 index 0000000..edc1927 --- /dev/null +++ b/server/internal/workspace/prebuild.go @@ -0,0 +1,377 @@ +package workspace + +import ( + "bufio" + "context" + "errors" + "fmt" + "log/slog" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + + "github.com/forgeutah/deuce/server/internal/agent/pirun/extension" +) + +// ErrPrebuildTagNotFound is returned when `devpod build` succeeded but its +// output did not carry the image tag line we parse. Callers degrade to the +// original from-scratch path rather than failing the session. +var ErrPrebuildTagNotFound = errors.New("could not parse prebuild image tag from devpod build output") + +// ansiRE strips the SGR colour codes devpod writes in its default `plain` +// log format, so tag parsing sees the bare text. +var ansiRE = regexp.MustCompile(`\x1b\[[0-9;]*[a-zA-Z]`) + +// prebuildImageRE captures the tag from devpod's closing build line: +// +// done Successfully build image deuce-prebuild:devpod-da04665bfb... +// +// Parsing a log line is fragile by nature, so every failure here is +// non-fatal: EnsurePrebuild returns an error and Create falls back to the +// original behaviour. A devpod upgrade that changes this wording costs the +// cache, not the session. +var prebuildImageRE = regexp.MustCompile(`Successfully build image (\S+)`) + +// validImageRef bounds what may reach `docker build -t` and `devpod up +// --devcontainer-image` argv. The value originates in devpod's stdout, which +// is not attacker-controlled in any path we know of — this is the same +// defence-in-depth posture as validContainerName, not the only barrier. +var validImageRef = regexp.MustCompile(`^[a-z0-9]+([._\-/][a-z0-9]+)*(:[0-9]+)?(/[a-zA-Z0-9._\-]+)*:[a-zA-Z0-9._\-]+$`) + +// devpodTagPrefix is the tag prefix devpod gives a prebuild image; the +// remainder is the hash of the devcontainer definition. Deuce republishes +// its own baked layer under deuceTagPrefix + the same hash, so the two +// images stay paired and both invalidate when the definition changes. +const ( + devpodTagPrefix = "devpod-" + deuceTagPrefix = "deuce-" +) + +// PrebuildResult describes the image a session should start from. +type PrebuildResult struct { + // Image is the tag to pass to `devpod up --devcontainer-image`. + Image string + + // ToolsBaked reports whether Image already contains Pi, the + // pi-subagents package and the ask-user extension. False means the + // caller must still run the over-ssh provisioning path — the + // devcontainer build was cached but the tooling layer was not. + ToolsBaked bool +} + +// SetPrebuildRepository turns on the prebuild cache for this manager. An +// empty repo (the default) leaves Create on its original from-scratch path. +func (m *Manager) SetPrebuildRepository(repo string) { + m.prebuildRepo = repo +} + +// PrebuildEnabled reports whether a prebuild repository is configured. +func (m *Manager) PrebuildEnabled() bool { + return m.prebuildRepo != "" +} + +// EnsurePrebuild makes sure a Deuce-baked devcontainer image exists for +// repoURL and returns the tag to start sessions from. +// +// Two steps, both cached: +// +// 1. `devpod build --repository --skip-push` builds the repo's own +// devcontainer once and tags it :devpod-, where is +// devpod's hash of the devcontainer definition. Re-running is cheap when +// the definition is unchanged (BuildKit cache hit), and produces a new +// hash when it changes — which is what gives the cache its staleness +// behaviour for free. +// 2. A thin Deuce layer bakes Pi, pi-subagents and the ask-user extension +// on top, tagged :deuce-. Skipped entirely when that tag +// already exists locally. +// +// Note this runs on every create, so the repo is still cloned once by +// `devpod build` — the win is skipping the image build and the Pi install, +// not the clone. Making step 1 conditional would mean caching the hash, +// which cannot be computed without the clone that produces it. +// +// A failure to bake is not a failure to start: the devpod prebuild image is +// returned with ToolsBaked false so the caller provisions over ssh as before. +func (m *Manager) EnsurePrebuild(ctx context.Context, repoURL string, logFn LogFunc) (PrebuildResult, error) { + if m.prebuildRepo == "" { + return PrebuildResult{}, errors.New("prebuild repository not configured") + } + + base, err := m.buildDevcontainerImage(ctx, repoURL, logFn) + if err != nil { + return PrebuildResult{}, err + } + + baked, err := bakedTag(base) + if err != nil { + return PrebuildResult{}, err + } + + if m.imageExists(ctx, baked) { + slog.Info("using cached baked prebuild image", "repo", repoURL, "image", baked) + if logFn != nil { + logFn(fmt.Sprintf("Using cached workspace image %s", baked)) + } + return PrebuildResult{Image: baked, ToolsBaked: true}, nil + } + + if err := m.bakeAgentTools(ctx, base, baked, logFn); err != nil { + // Degrade, don't fail: the devcontainer build is still cached, and + // the caller's over-ssh provisioning fills in the tooling. + slog.Warn("baking agent tools into prebuild image failed; falling back to over-ssh provisioning", + "repo", repoURL, "base", base, "error", err) + if logFn != nil { + logFn("WARNING: could not bake Pi into the workspace image — falling back to installing it after start") + } + return PrebuildResult{Image: base, ToolsBaked: false}, nil + } + + return PrebuildResult{Image: baked, ToolsBaked: true}, nil +} + +// buildDevcontainerImage runs `devpod build` and returns the tag it reports. +func (m *Manager) buildDevcontainerImage(ctx context.Context, repoURL string, logFn LogFunc) (string, error) { + args := devpodBuildArgs(repoURL, m.prebuildRepo, m.provider) + + slog.Info("building devcontainer prebuild image", "repo", repoURL, "repository", m.prebuildRepo) + if logFn != nil { + logFn("Preparing workspace image (cached after the first build)...") + } + + cmd := exec.CommandContext(ctx, m.bin, args...) + // Same clone-time credential story as Create — `devpod build` clones the + // repo too, so private repos need the isolated credential store here. + if gitEnv := m.gitCredentialEnv(); gitEnv != nil { + cmd.Env = append(os.Environ(), gitEnv...) + } + + stdout, err := cmd.StdoutPipe() + if err != nil { + return "", fmt.Errorf("stdout pipe: %w", err) + } + cmd.Stderr = cmd.Stdout + + if err := cmd.Start(); err != nil { + return "", fmt.Errorf("devpod build start: %w", err) + } + + var tag string + scanner := bufio.NewScanner(stdout) + scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) + for scanner.Scan() { + line := ansiRE.ReplaceAllString(scanner.Text(), "") + slog.Debug("devpod build output", "repo", repoURL, "line", line) + if logFn != nil { + logFn(line) + } + // Last match wins: devpod names several intermediate images during + // the build and reports the final one on its closing line. + if match := prebuildImageRE.FindStringSubmatch(line); match != nil { + tag = match[1] + } + } + + if err := cmd.Wait(); err != nil { + return "", fmt.Errorf("devpod build failed: %w", err) + } + if tag == "" || !validImageRef.MatchString(tag) { + return "", fmt.Errorf("%w (got %q)", ErrPrebuildTagNotFound, tag) + } + + slog.Info("devcontainer prebuild image ready", "repo", repoURL, "image", tag) + return tag, nil +} + +// devpodBuildArgs assembles the `devpod build` argv. --skip-push keeps the +// resulting tag local to the Docker daemon: the image is consumed by tag via +// --devcontainer-image, so no registry is involved. +func devpodBuildArgs(repoURL, repository, provider string) []string { + args := []string{"build", repoURL, "--repository", repository, "--skip-push"} + if provider != "" { + args = append(args, "--provider", provider) + } + return args +} + +// bakedTag maps devpod's prebuild tag to the tag Deuce bakes its own layer +// under, preserving the definition hash so both invalidate together. +func bakedTag(prebuild string) (string, error) { + idx := strings.LastIndex(prebuild, ":") + if idx < 0 { + return "", fmt.Errorf("prebuild image %q has no tag", prebuild) + } + repo, tag := prebuild[:idx], prebuild[idx+1:] + hash, ok := strings.CutPrefix(tag, devpodTagPrefix) + if !ok { + return "", fmt.Errorf("prebuild image tag %q does not start with %q", tag, devpodTagPrefix) + } + return repo + ":" + deuceTagPrefix + hash, nil +} + +// imageExists reports whether the Docker daemon already holds the tag. +func (m *Manager) imageExists(ctx context.Context, image string) bool { + _, err := m.runner(ctx, "docker", "image", "inspect", image) + return err == nil +} + +// imageUser resolves the user the baked layer's RUN steps should execute as, +// from the same merged `devcontainer.metadata` label ContainerUser reads. +// Installing as root would put Pi in /root, where the session's remoteUser +// cannot reach it. Empty means "leave the image's USER alone". +func (m *Manager) imageUser(ctx context.Context, image string) string { + out, err := m.runner(ctx, "docker", "image", "inspect", + "--format", `{{index .Config.Labels "devcontainer.metadata"}}`, image) + if err != nil { + slog.Warn("could not read devcontainer metadata from prebuild image", "image", image, "error", err) + return "" + } + return execUserFromMetadata(strings.TrimSpace(string(out))) +} + +// imageDefaultUser returns the image's own USER directive, so the baked +// layer can restore it rather than silently changing the image's default. +func (m *Manager) imageDefaultUser(ctx context.Context, image string) string { + out, err := m.runner(ctx, "docker", "image", "inspect", "--format", `{{.Config.User}}`, image) + if err != nil { + return "" + } + user := strings.TrimSpace(string(out)) + if !validExecUser.MatchString(user) { + return "" + } + return user +} + +// bakeAgentTools builds the thin Deuce layer on top of base and tags it +// baked. The build context is generated on the fly from the same Go +// constants the over-ssh installers use, so the two paths cannot drift. +func (m *Manager) bakeAgentTools(ctx context.Context, base, baked string, logFn LogFunc) error { + runAs := m.imageUser(ctx, base) + if runAs == "" { + // No declared remoteUser: install as whoever the image runs as. + runAs = m.imageDefaultUser(ctx, base) + } + if runAs == "" { + runAs = "root" + } + finalUser := m.imageDefaultUser(ctx, base) + if finalUser == "" { + finalUser = "root" + } + + dir, err := os.MkdirTemp("", "deuce-workspace-image-") + if err != nil { + return fmt.Errorf("create build context: %w", err) + } + defer os.RemoveAll(dir) + + files := map[string]string{ + "Dockerfile": workspaceImageDockerfile, + "pi-install.sh": piInstallScript, + extension.AskUserFilename: extension.AskUser, + } + for name, content := range files { + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { + return fmt.Errorf("write %s: %w", name, err) + } + } + + slog.Info("baking agent tools into prebuild image", "base", base, "baked", baked, "user", runAs) + if logFn != nil { + logFn("Baking Pi and agent tooling into the workspace image...") + } + + args := []string{ + "build", + "--build-arg", "BASE=" + base, + "--build-arg", "REMOTE_USER=" + runAs, + "--build-arg", "FINAL_USER=" + finalUser, + "--build-arg", "PI_SUBAGENTS=" + PiSubagentsPackage, + "--build-arg", "ASK_USER_FILE=" + extension.AskUserFilename, + "-t", baked, + dir, + } + cmd := exec.CommandContext(ctx, "docker", args...) + stdout, err := cmd.StdoutPipe() + if err != nil { + return fmt.Errorf("stdout pipe: %w", err) + } + cmd.Stderr = cmd.Stdout + if err := cmd.Start(); err != nil { + return fmt.Errorf("docker build start: %w", err) + } + scanner := bufio.NewScanner(stdout) + scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) + for scanner.Scan() { + line := scanner.Text() + slog.Debug("workspace image build output", "line", line) + if logFn != nil { + logFn(line) + } + } + if err := cmd.Wait(); err != nil { + return fmt.Errorf("docker build failed: %w", err) + } + + if logFn != nil { + logFn(fmt.Sprintf("Workspace image ready: %s", baked)) + } + return nil +} + +// workspaceImageDockerfile bakes the agent harness onto a devpod-built +// devcontainer image. It is a Go constant rather than a file under deploy/ +// so a deployed `deuce` binary carries it — the plan named a deploy/ +// directory, but that would make the running server depend on the repo +// layout being present next to it. +// +// HOME is resolved per RUN from the passwd database: Docker's USER directive +// changes the uid but not $HOME, so a bare "$HOME" here would resolve to +// root's home and install Pi where the session user cannot reach it. +const workspaceImageDockerfile = `# syntax=docker/dockerfile:1 +ARG BASE +FROM ${BASE} + +ARG REMOTE_USER=root +ARG FINAL_USER=root +ARG PI_SUBAGENTS +ARG ASK_USER_FILE + +USER root +COPY pi-install.sh /tmp/deuce-build/pi-install.sh +COPY ${ASK_USER_FILE} /tmp/deuce-build/${ASK_USER_FILE} +RUN chmod 0755 /tmp/deuce-build/pi-install.sh && chmod -R a+rX /tmp/deuce-build + +USER ${REMOTE_USER} + +# Pi itself, via the same script the over-ssh installer uses. +RUN set -eu; \ + H="$(getent passwd "$(id -un)" 2>/dev/null | cut -d: -f6 || true)"; \ + [ -n "$H" ] || H="$HOME"; \ + [ -n "$H" ] || H=/root; \ + export HOME="$H"; \ + sh /tmp/deuce-build/pi-install.sh + +# The subagents package, registered in Pi's own settings. +RUN set -eu; \ + H="$(getent passwd "$(id -un)" 2>/dev/null | cut -d: -f6 || true)"; \ + [ -n "$H" ] || H="$HOME"; \ + [ -n "$H" ] || H=/root; \ + export HOME="$H"; \ + export PATH="$H/.local/bin:$PATH"; \ + pi install "${PI_SUBAGENTS}" + +# The ask-user extension, in Pi's auto-discovery path. +RUN set -eu; \ + H="$(getent passwd "$(id -un)" 2>/dev/null | cut -d: -f6 || true)"; \ + [ -n "$H" ] || H="$HOME"; \ + [ -n "$H" ] || H=/root; \ + mkdir -p "$H/.pi/agent/extensions"; \ + cp "/tmp/deuce-build/${ASK_USER_FILE}" "$H/.pi/agent/extensions/${ASK_USER_FILE}" + +USER root +RUN rm -rf /tmp/deuce-build +USER ${FINAL_USER} +` diff --git a/server/internal/workspace/prebuild_e2e_test.go b/server/internal/workspace/prebuild_e2e_test.go new file mode 100644 index 0000000..cc58c38 --- /dev/null +++ b/server/internal/workspace/prebuild_e2e_test.go @@ -0,0 +1,108 @@ +package workspace + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// TestEnsurePrebuild_EndToEnd exercises the real thing: `devpod build`, the +// baked Deuce layer, the cache hit on a second call, and `devpod up +// --devcontainer-image` producing a container that actually has Pi on PATH +// as the devcontainer's remoteUser. +// +// Opt-in because it shells out to devpod and docker, pulls a base image, and +// downloads Node and Pi over the network — minutes, not milliseconds. Run it +// after touching prebuild.go or the baked Dockerfile: +// +// DEUCE_PREBUILD_E2E=1 go test ./internal/workspace/ -run EndToEnd -v -timeout 20m +func TestEnsurePrebuild_EndToEnd(t *testing.T) { + if os.Getenv("DEUCE_PREBUILD_E2E") == "" { + t.Skip("set DEUCE_PREBUILD_E2E=1 to run (shells out to devpod + docker, needs network)") + } + for _, bin := range []string{"devpod", "docker"} { + if _, err := exec.LookPath(bin); err != nil { + t.Skipf("%s not on PATH", bin) + } + } + + // A devcontainer with a build step: devpod skips prebuild entirely for a + // plain "image" devcontainer, so an image-only fixture would pass + // without exercising anything. + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, ".devcontainer"), 0o755); err != nil { + t.Fatal(err) + } + write := func(name, content string) { + if err := os.WriteFile(filepath.Join(dir, ".devcontainer", name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + write("Dockerfile", "FROM mcr.microsoft.com/devcontainers/base:ubuntu\nRUN echo e2e > /etc/deuce-e2e-marker\n") + write("devcontainer.json", `{"name":"deuce-e2e","build":{"dockerfile":"Dockerfile"},"remoteUser":"vscode"}`) + + const repository = "deuce-prebuild-e2e" + m := NewManager("devpod", "docker", "") + m.SetPrebuildRepository(repository) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Minute) + defer cancel() + + logFn := func(line string) { t.Log(line) } + + res, err := m.EnsurePrebuild(ctx, dir, logFn) + if err != nil { + t.Fatalf("EnsurePrebuild: %v", err) + } + t.Cleanup(func() { + _ = exec.Command("docker", "image", "rm", "-f", res.Image).Run() + }) + if !res.ToolsBaked { + t.Fatal("expected ToolsBaked — the baked layer failed and silently degraded") + } + if !strings.Contains(res.Image, ":"+deuceTagPrefix) { + t.Fatalf("expected a Deuce-tagged image, got %q", res.Image) + } + + // Second call must hit the cache: same tag, and no rebuild. + second, err := m.EnsurePrebuild(ctx, dir, logFn) + if err != nil { + t.Fatalf("second EnsurePrebuild: %v", err) + } + if second.Image != res.Image || !second.ToolsBaked { + t.Errorf("cache miss on second call: first=%+v second=%+v", res, second) + } + + // The payoff: a container started from the baked image must have Pi + // runnable as the remoteUser, with no over-ssh install. + const wsID = "deuce-prebuild-e2e-ws" + t.Cleanup(func() { + _ = exec.Command("devpod", "delete", wsID, "--force", "--ignore-not-found").Run() + }) + if _, err := m.Create(ctx, wsID, dir, logFn); err != nil { + t.Fatalf("Create from baked image: %v", err) + } + + checks := map[string]string{ + "pi on PATH": piLoginShell("pi --version"), + "ask-user extension": `test -f "$HOME/.pi/agent/extensions/ask-user.ts" && echo present`, + "remote user": "whoami", + "base layer intact": "cat /etc/deuce-e2e-marker", + } + for name, cmd := range checks { + out, err := m.ExecInWorkspace(ctx, wsID, cmd).CombinedOutput() + got := strings.TrimSpace(string(out)) + if err != nil { + t.Errorf("%s: %v (output: %s)", name, err, got) + continue + } + t.Logf("%s -> %s", name, got) + if got == "" { + t.Errorf("%s produced no output", name) + } + } +} diff --git a/server/internal/workspace/prebuild_test.go b/server/internal/workspace/prebuild_test.go new file mode 100644 index 0000000..c295c89 --- /dev/null +++ b/server/internal/workspace/prebuild_test.go @@ -0,0 +1,329 @@ +package workspace + +import ( + "context" + "errors" + "slices" + "strings" + "testing" +) + +// devpodBuildOutput is a verbatim capture of devpod v0.6.15's closing lines, +// ANSI colour codes and all. Parsing real output rather than a cleaned-up +// approximation is the point: the colour codes are exactly what would break +// a naive prefix match. +const devpodBuildOutput = "\x1b[0;1;37m21:14:26 \x1b[0m\x1b[0;1;36minfo \x1b[0m#6 naming to docker.io/library/deuce-prebuild-probe:devpod-da04665bfb6d8267ff56dcd9e6483d75 done\n" + + "\x1b[0;1;37m21:14:26 \x1b[0m\x1b[0;1;32mdone \x1b[0mSuccessfully build image deuce-prebuild-probe:devpod-da04665bfb6d8267ff56dcd9e6483d75\n" + + "\x1b[0;1;37m21:14:26 \x1b[0m\x1b[0;1;36minfo \x1b[0mDeleting container...\n" + +// TestPrebuildImageRE_ParsesRealDevpodOutput locks in tag extraction against +// the actual log format, including the ANSI stripping it depends on. +func TestPrebuildImageRE_ParsesRealDevpodOutput(t *testing.T) { + var got string + for _, raw := range strings.Split(devpodBuildOutput, "\n") { + line := ansiRE.ReplaceAllString(raw, "") + if m := prebuildImageRE.FindStringSubmatch(line); m != nil { + got = m[1] + } + } + want := "deuce-prebuild-probe:devpod-da04665bfb6d8267ff56dcd9e6483d75" + if got != want { + t.Errorf("parsed tag = %q, want %q", got, want) + } + if !validImageRef.MatchString(got) { + t.Errorf("parsed tag %q failed validImageRef", got) + } +} + +// TestPrebuildImageRE_NoMatchLeavesEmpty confirms unrelated output does not +// yield a bogus tag — the caller treats empty as ErrPrebuildTagNotFound and +// falls back rather than passing garbage to docker. +func TestPrebuildImageRE_NoMatchLeavesEmpty(t *testing.T) { + for _, line := range []string{ + "info Building devcontainer...", + "error failed to solve: process did not complete successfully", + "", + } { + if m := prebuildImageRE.FindStringSubmatch(line); m != nil { + t.Errorf("line %q unexpectedly matched, got %q", line, m[1]) + } + } +} + +func TestBakedTag(t *testing.T) { + tests := []struct { + name string + prebuild string + want string + wantErr bool + }{ + { + name: "bare repository", + prebuild: "deuce-prebuild:devpod-da04665bfb6d8267ff56dcd9e6483d75", + want: "deuce-prebuild:deuce-da04665bfb6d8267ff56dcd9e6483d75", + }, + { + name: "registry path", + prebuild: "ghcr.io/forgeutah/deuce-prebuild:devpod-abc123", + want: "ghcr.io/forgeutah/deuce-prebuild:deuce-abc123", + }, + { + // The colon in a host:port must not be mistaken for the tag + // separator — LastIndex is what makes this work. + name: "registry with port", + prebuild: "localhost:5000/deuce-prebuild:devpod-abc123", + want: "localhost:5000/deuce-prebuild:deuce-abc123", + }, + { + name: "no tag at all", + prebuild: "deuce-prebuild", + wantErr: true, + }, + { + // devpod changed its tagging scheme: better to fail and fall + // back than to bake onto an image we cannot key correctly. + name: "unexpected tag prefix", + prebuild: "deuce-prebuild:latest", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := bakedTag(tt.prebuild) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got %q", got) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Errorf("bakedTag(%q) = %q, want %q", tt.prebuild, got, tt.want) + } + }) + } +} + +// TestBakedTag_PreservesHash is the staleness guarantee: the baked tag must +// carry the devcontainer-definition hash, so a definition change yields a new +// devpod tag AND a new baked tag, while an ordinary code push changes neither. +func TestBakedTag_PreservesHash(t *testing.T) { + before, err := bakedTag("repo:devpod-hash-one") + if err != nil { + t.Fatal(err) + } + after, err := bakedTag("repo:devpod-hash-two") + if err != nil { + t.Fatal(err) + } + if before == after { + t.Fatal("different definition hashes produced the same baked tag") + } + same, err := bakedTag("repo:devpod-hash-one") + if err != nil { + t.Fatal(err) + } + if same != before { + t.Errorf("same definition hash produced different baked tags: %q vs %q", before, same) + } +} + +func TestValidImageRef(t *testing.T) { + valid := []string{ + "deuce-prebuild:devpod-abc123", + "ghcr.io/forgeutah/deuce-prebuild:deuce-abc123", + "localhost:5000/deuce-prebuild:devpod-abc", + } + for _, ref := range valid { + if !validImageRef.MatchString(ref) { + t.Errorf("expected %q to be valid", ref) + } + } + + invalid := []string{ + "", + "no-tag", + "--build-arg=evil:tag", // flag-shaped + "repo:tag with space", // shell-meta + "repo:tag;rm -rf /", // shell-meta + "repo:tag\nsecond", // embedded newline + "repo:$(hostile)", // command substitution + } + for _, ref := range invalid { + if validImageRef.MatchString(ref) { + t.Errorf("expected %q to be rejected", ref) + } + } +} + +// TestDevpodUpArgs covers the plan's "argv assembly with and without +// prebuild" scenario: an empty image must leave the command byte-identical +// to the pre-prebuild behaviour. +func TestDevpodUpArgs(t *testing.T) { + tests := []struct { + name string + provider string + image string + want []string + }{ + { + name: "prebuild disabled keeps the original argv", + provider: "docker", + image: "", + want: []string{"up", "https://example.com/r.git", "--id", "ws1", "--ide", "none", "--provider", "docker"}, + }, + { + name: "prebuild enabled appends the image override", + provider: "docker", + image: "repo:deuce-abc", + want: []string{"up", "https://example.com/r.git", "--id", "ws1", "--ide", "none", "--provider", "docker", "--devcontainer-image", "repo:deuce-abc"}, + }, + { + name: "empty provider is omitted", + provider: "", + image: "repo:deuce-abc", + want: []string{"up", "https://example.com/r.git", "--id", "ws1", "--ide", "none", "--devcontainer-image", "repo:deuce-abc"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := devpodUpArgs("ws1", "https://example.com/r.git", tt.provider, tt.image) + if !slices.Equal(got, tt.want) { + t.Errorf("got %v\nwant %v", got, tt.want) + } + }) + } +} + +func TestDevpodBuildArgs(t *testing.T) { + got := devpodBuildArgs("https://example.com/r.git", "deuce-prebuild", "docker") + want := []string{"build", "https://example.com/r.git", "--repository", "deuce-prebuild", "--skip-push", "--provider", "docker"} + if !slices.Equal(got, want) { + t.Errorf("got %v\nwant %v", got, want) + } + + // --skip-push is what keeps the cache local; losing it would make every + // create attempt a registry push. + if !slices.Contains(got, "--skip-push") { + t.Error("expected --skip-push in devpod build argv") + } +} + +// TestEnsurePrebuild_DisabledIsAnError confirms the guard: callers must check +// prebuildRepo before calling, and a misconfigured manager cannot silently +// shell out to `devpod build` with an empty --repository. +func TestEnsurePrebuild_DisabledIsAnError(t *testing.T) { + m := NewManager("devpod", "docker", "") + if m.PrebuildEnabled() { + t.Fatal("a fresh manager should have the prebuild cache off") + } + if _, err := m.EnsurePrebuild(context.Background(), "https://example.com/r.git", nil); err == nil { + t.Fatal("expected an error when no prebuild repository is configured") + } +} + +func TestSetPrebuildRepository(t *testing.T) { + m := NewManager("devpod", "docker", "") + m.SetPrebuildRepository("deuce-prebuild") + if !m.PrebuildEnabled() { + t.Error("expected prebuild to be enabled after SetPrebuildRepository") + } + m.SetPrebuildRepository("") + if m.PrebuildEnabled() { + t.Error("expected an empty repository to disable prebuild") + } +} + +// TestImageExists_UsesRunnerSeam checks the cache-hit decision without +// shelling out to docker, and confirms a docker failure reads as "absent" +// (which costs a rebuild) rather than as "present" (which would pass a +// nonexistent image to devpod up). +func TestImageExists_UsesRunnerSeam(t *testing.T) { + var gotArgs []string + m := NewManager("devpod", "docker", "") + + m.runner = func(_ context.Context, name string, args ...string) ([]byte, error) { + gotArgs = append([]string{name}, args...) + return []byte("[]"), nil + } + if !m.imageExists(context.Background(), "repo:deuce-abc") { + t.Error("expected imageExists to report true when docker inspect succeeds") + } + want := []string{"docker", "image", "inspect", "repo:deuce-abc"} + if !slices.Equal(gotArgs, want) { + t.Errorf("got %v\nwant %v", gotArgs, want) + } + + m.runner = func(_ context.Context, _ string, _ ...string) ([]byte, error) { + return nil, errors.New("No such image") + } + if m.imageExists(context.Background(), "repo:deuce-abc") { + t.Error("expected imageExists to report false when docker inspect fails") + } +} + +// TestImageUser_ResolvesRemoteUser covers the bug this guards against: +// installing Pi as root when the session runs as the devcontainer's +// remoteUser would put the binary somewhere the agent cannot reach. +func TestImageUser_ResolvesRemoteUser(t *testing.T) { + m := NewManager("devpod", "docker", "") + m.runner = func(_ context.Context, _ string, _ ...string) ([]byte, error) { + return []byte(`[{"id":"ghcr.io/devcontainers/features/git:1"},{"remoteUser":"vscode"}]` + "\n"), nil + } + if got := m.imageUser(context.Background(), "repo:devpod-abc"); got != "vscode" { + t.Errorf("imageUser = %q, want %q", got, "vscode") + } + + // A docker failure must degrade to "" so bakeAgentTools falls back to + // the image's own USER rather than guessing. + m.runner = func(_ context.Context, _ string, _ ...string) ([]byte, error) { + return nil, errors.New("no such image") + } + if got := m.imageUser(context.Background(), "repo:devpod-abc"); got != "" { + t.Errorf("imageUser on error = %q, want empty", got) + } +} + +// TestWorkspaceImageDockerfile_ResolvesHomePerRun guards the subtlest part of +// the baked layer: Docker's USER directive changes the uid but not $HOME, so +// every RUN must resolve HOME from the passwd database. A regression here +// installs Pi into /root and the agent silently fails to launch. +func TestWorkspaceImageDockerfile_ResolvesHomePerRun(t *testing.T) { + runCount := strings.Count(workspaceImageDockerfile, "\nRUN ") + homeResolutions := strings.Count(workspaceImageDockerfile, `getent passwd "$(id -un)"`) + if homeResolutions < 3 { + t.Errorf("expected each install RUN to resolve HOME from passwd, found %d resolutions across %d RUN steps", + homeResolutions, runCount) + } + if strings.Contains(workspaceImageDockerfile, "ARG BASE") == false { + t.Error("Dockerfile must accept a BASE build arg") + } + // The layer must drop out of root before installing, otherwise the + // files land with the wrong ownership. + lines := strings.Split(workspaceImageDockerfile, "\n") + userLine, installLine := -1, -1 + for i, l := range lines { + trimmed := strings.TrimSpace(l) + if userLine < 0 && trimmed == "USER ${REMOTE_USER}" { + userLine = i + } + // Match the invocation, not the COPY that puts the script in place. + if installLine < 0 && strings.HasPrefix(trimmed, "sh /tmp/deuce-build/pi-install.sh") { + installLine = i + } + } + if userLine < 0 { + t.Fatal("Dockerfile never switches to ${REMOTE_USER}") + } + if installLine < 0 { + t.Fatal("Dockerfile never invokes pi-install.sh") + } + if userLine > installLine { + t.Errorf("USER ${REMOTE_USER} (line %d) must precede the Pi install (line %d), else Pi installs as root", + userLine, installLine) + } +} From f4713fedd9546ff8eaabf299b8ea579d805abbeb Mon Sep 17 00:00:00 2001 From: Clint Berry Date: Tue, 28 Jul 2026 21:40:28 +0000 Subject: [PATCH 2/4] feat(workspace): carry ~/.vscode-server across container recreates VS Code Remote-SSH installs a ~120MB server payload into the container's own filesystem, so every container recreate re-downloaded it. DEUCE_VSCODE_SERVER_CACHE_DIR (empty by default = off) copies the tree out to a host cache before an action that destroys the container, and copies it back in after the container is recreated. The plan called for a per-user named volume; DevPod exposes no way to add one. `devpod up` has no mount flag and the docker provider's options are only DOCKER_BUILDER/DOCKER_HOST/DOCKER_PATH/INACTIVITY_TIMEOUT, so mounts can only come from the repo's own devcontainer.json, which Deuce does not control. `docker cp` is the mechanism actually available, and it turns a network download into a local disk copy. The cache is keyed by workspace rather than by user. A workspace's container is already shared by every member of its session, so a per-workspace cache adds no new reach; keying it per user would copy one user's extension state, including whatever credentials their extensions have stored, into a container other session members hold a shell on. Saves stage into a sibling .partial directory and are promoted by rename, so an interrupted copy cannot leave a half-written tree that a later restore would push into a container as if it were complete. Caches are purged when their workspace is deleted. Home is resolved from the passwd database rather than $HOME, which `docker exec` does not set. Every operation is best-effort: a failure costs a re-download, not a session. Verified end-to-end (opt-in DEUCE_PREBUILD_E2E test): payload saved, the container deleted and recreated, confirmed absent from the fresh container, then restored owned by the vscode remoteUser and writable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014orMLVuaREojZaJ6mzXvtv --- CLAUDE.md | 13 +- server/internal/config/config.go | 15 ++ server/internal/config/config_test.go | 32 +++ server/internal/handler/sessions.go | 1 + server/internal/handler/workspace.go | 31 +++ server/internal/server/server.go | 1 + server/internal/workspace/manager.go | 10 + .../internal/workspace/prebuild_e2e_test.go | 97 ++++++++ server/internal/workspace/vscode_cache.go | 233 ++++++++++++++++++ .../internal/workspace/vscode_cache_test.go | 197 +++++++++++++++ 10 files changed, 629 insertions(+), 1 deletion(-) create mode 100644 server/internal/workspace/vscode_cache.go create mode 100644 server/internal/workspace/vscode_cache_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 3412b5c..c485e4c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -100,6 +100,13 @@ DEVPOD_PROVIDER= # DevPod provider (empty = default) # install. Tags stay local to the Docker daemon; no registry is required. DEUCE_PREBUILD_REPOSITORY= +# Carries a workspace's ~/.vscode-server tree across container recreates so +# VS Code Remote-SSH does not re-download its ~120MB server payload. Empty +# (default) = off. Must be an absolute path. Budget ~120MB per workspace that +# has been opened in VS Code; an entry is removed when its workspace is +# deleted. Keyed by workspace, not by user — see the VS Code section below. +DEUCE_VSCODE_SERVER_CACHE_DIR= + # Agent backend. Pi (pi.dev) runs in --mode rpc inside each session's DevPod # container, driven over a persistent JSONL channel — one process per session. # PiProvider/PiModel select the Pi backend (v1 runs Claude models through Pi). @@ -249,7 +256,11 @@ Devcontainers used with "Open in VS Code" must include: - `openssh-sftp-server` (Debian: `/usr/lib/openssh/sftp-server`) — required for SFTP-based file operations - A **glibc-compatible base image** — VS Code Remote requires glibc ≥ 2.17. Alpine + musl needs `apk add gcompat`. -`~/.vscode-server` lives in the container's own filesystem; a fresh ~120MB download fires on every container recreate. Per-user named volume caching is a v2 follow-up. +`~/.vscode-server` lives in the container's own filesystem, so a fresh ~120MB download would fire on every container recreate. Setting `DEUCE_VSCODE_SERVER_CACHE_DIR` avoids that: Deuce copies the tree out to a host cache before stop/rebuild and copies it back in after the container is recreated (`server/internal/workspace/vscode_cache.go`). + +A named volume would be the obvious mechanism, but DevPod exposes no way to add one — `devpod up` has no mount flag and the docker provider's options are only `DOCKER_BUILDER`/`DOCKER_HOST`/`DOCKER_PATH`/`INACTIVITY_TIMEOUT`. Mounts can only come from the repo's own `devcontainer.json`, which Deuce does not control, hence `docker cp`. + +The cache is keyed by **workspace**, not by user. A workspace's container is already shared by every member of its session, so a per-workspace cache adds no new reach; keying it per user would copy one user's extension state — including credentials their extensions have stored — into a container other session members hold a shell on. ## Documented Solutions diff --git a/server/internal/config/config.go b/server/internal/config/config.go index 0837bd5..1d84f72 100644 --- a/server/internal/config/config.go +++ b/server/internal/config/config.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io/fs" + "path/filepath" "regexp" "slices" "strings" @@ -55,6 +56,14 @@ type Config struct { // `--prebuild-repository` (which only ever does a registry lookup). PrebuildRepository string `env:"DEUCE_PREBUILD_REPOSITORY" envDefault:""` + // VSCodeCacheDir enables carrying a workspace's ~/.vscode-server tree + // across container recreates, so VS Code Remote-SSH does not + // re-download its ~120MB server payload every time. Empty (the + // default) disables it. Budget roughly 120MB of disk per workspace + // that has been opened in VS Code; entries are removed when their + // workspace is deleted. + VSCodeCacheDir string `env:"DEUCE_VSCODE_SERVER_CACHE_DIR" envDefault:""` + // PiProvider/PiModel configure the Pi agent backend (the sole harness); // v1 runs Claude models through Pi. PiProvider string `env:"DEUCE_PI_PROVIDER" envDefault:"anthropic"` @@ -177,6 +186,12 @@ func (c *Config) Validate() error { return fmt.Errorf("DEUCE_PREBUILD_REPOSITORY=%q is not a valid Docker repository name (lowercase, no tag — Deuce appends the devcontainer hash as the tag)", c.PrebuildRepository) } + // A relative cache root would resolve against the server's working + // directory, which differs between `make dev` and a deployed unit. + if c.VSCodeCacheDir != "" && !filepath.IsAbs(c.VSCodeCacheDir) { + return fmt.Errorf("DEUCE_VSCODE_SERVER_CACHE_DIR=%q must be an absolute path", c.VSCodeCacheDir) + } + if c.AuthMode == AuthModeProxy { return c.validateProxyMode(origins) } diff --git a/server/internal/config/config_test.go b/server/internal/config/config_test.go index 694cfeb..2bbd821 100644 --- a/server/internal/config/config_test.go +++ b/server/internal/config/config_test.go @@ -366,3 +366,35 @@ func TestValidate_PrebuildRepositoryRejected(t *testing.T) { } } } + +func TestValidate_VSCodeCacheDirMustBeAbsolute(t *testing.T) { + // A relative root would resolve against the server's working directory, + // which differs between `make dev` and a deployed unit — the cache would + // silently land somewhere different depending on how deuce was started. + for _, dir := range []string{"relative/path", "./cache", "cache"} { + cfg := &Config{ + AuthMode: AuthModeDev, + WSAllowedOrigins: "localhost:4000", + VSCodeCacheDir: dir, + } + err := cfg.Validate() + if err == nil { + t.Errorf("VSCodeCacheDir=%q should be rejected", dir) + continue + } + if !strings.Contains(err.Error(), "DEUCE_VSCODE_SERVER_CACHE_DIR") { + t.Errorf("error for %q should name the env var: %v", dir, err) + } + } + + for _, dir := range []string{"", "/var/lib/deuce/vscode"} { + cfg := &Config{ + AuthMode: AuthModeDev, + WSAllowedOrigins: "localhost:4000", + VSCodeCacheDir: dir, + } + if err := cfg.Validate(); err != nil { + t.Errorf("VSCodeCacheDir=%q should validate: %v", dir, err) + } + } +} diff --git a/server/internal/handler/sessions.go b/server/internal/handler/sessions.go index e0019cf..6fcf9af 100644 --- a/server/internal/handler/sessions.go +++ b/server/internal/handler/sessions.go @@ -597,6 +597,7 @@ func (h *Handler) startWorkspace(sessionID uuid.UUID, workspaceID, repoURL strin // runs on every start/rebuild so older containers migrate. Skipped // when the workspace image already carries them. h.provisionAgentToolsIfNeeded(ctx, workspaceID, prebuilt, logFn) + h.restoreVSCodeServer(ctx, workspaceID, logFn) newStatus = "ready" } diff --git a/server/internal/handler/workspace.go b/server/internal/handler/workspace.go index d94ac87..f249a40 100644 --- a/server/internal/handler/workspace.go +++ b/server/internal/handler/workspace.go @@ -50,6 +50,28 @@ func (h *Handler) provisionAgentToolsIfNeeded(ctx context.Context, workspaceID s h.provisionAgentTools(ctx, workspaceID, logFn) } +// saveVSCodeServer and restoreVSCodeServer carry the VS Code Remote server +// payload across container recreates. Both are best-effort: the cache is an +// optimisation, and losing it costs a re-download, not a session. Both are +// no-ops when DEUCE_VSCODE_SERVER_CACHE_DIR is unset. +func (h *Handler) saveVSCodeServer(ctx context.Context, workspaceID string, logFn workspace.LogFunc) { + if !h.workspaces.VSCodeCacheEnabled() { + return + } + if err := h.workspaces.SaveVSCodeServer(ctx, workspaceID, logFn); err != nil { + slog.Warn("failed to cache vscode-server payload", "workspace", workspaceID, "error", err) + } +} + +func (h *Handler) restoreVSCodeServer(ctx context.Context, workspaceID string, logFn workspace.LogFunc) { + if !h.workspaces.VSCodeCacheEnabled() { + return + } + if err := h.workspaces.RestoreVSCodeServer(ctx, workspaceID, logFn); err != nil { + slog.Warn("failed to restore vscode-server payload", "workspace", workspaceID, "error", err) + } +} + // workspaceAction names the four lifecycle operations the user can trigger. // Each one maps to a transitional workspace_status that the handler writes // synchronously, then a devpod CLI call in a tracked background goroutine, @@ -246,12 +268,15 @@ func (h *Handler) runWorkspaceAction(sessionID uuid.UUID, workspaceID, repoURL s // before agent support — or where a prior install failed — pick up // Pi + the ask-user extension. The installers are idempotent. h.provisionAgentToolsIfNeeded(ctx, workspaceID, prebuilt, logFn) + h.restoreVSCodeServer(ctx, workspaceID, logFn) newStatus = "ready" } else { newStatus = "failed" } case actionStop: + // Save before the container goes away, not after. + h.saveVSCodeServer(ctx, workspaceID, logFn) actErr = h.workspaces.Stop(ctx, workspaceID) if actErr == nil { newStatus = "stopped" @@ -264,6 +289,7 @@ func (h *Handler) runWorkspaceAction(sessionID uuid.UUID, workspaceID, repoURL s // output is not streamed via logFn — the user will see logs once // Create begins. Documented in the plan's Rebuild risk note. var rebuilt workspace.PrebuildResult + h.saveVSCodeServer(ctx, workspaceID, logFn) if delErr := h.workspaces.Delete(ctx, workspaceID); delErr != nil { actErr = fmt.Errorf("rebuild delete: %w", delErr) } else { @@ -271,6 +297,7 @@ func (h *Handler) runWorkspaceAction(sessionID uuid.UUID, workspaceID, repoURL s } if actErr == nil { h.provisionAgentToolsIfNeeded(ctx, workspaceID, rebuilt, logFn) + h.restoreVSCodeServer(ctx, workspaceID, logFn) newStatus = "ready" } else { newStatus = "failed" @@ -279,6 +306,10 @@ func (h *Handler) runWorkspaceAction(sessionID uuid.UUID, workspaceID, repoURL s case actionDelete: actErr = h.workspaces.Delete(ctx, workspaceID) if actErr == nil { + // The cache belongs to the workspace; it must not outlive it. + if err := h.workspaces.PurgeVSCodeServer(workspaceID); err != nil { + slog.Warn("failed to purge vscode-server cache", "workspace", workspaceID, "error", err) + } newStatus = "missing" } else { newStatus = "failed" diff --git a/server/internal/server/server.go b/server/internal/server/server.go index bde705b..b5ecd28 100644 --- a/server/internal/server/server.go +++ b/server/internal/server/server.go @@ -124,6 +124,7 @@ func (s *Server) Router() http.Handler { // Only this manager creates workspaces; the sshproxy and reconciler // managers in main.go never call Create, so they need no prebuild repo. wm.SetPrebuildRepository(s.cfg.PrebuildRepository) + wm.SetVSCodeCacheDir(s.cfg.VSCodeCacheDir) if s.cfg.PrebuildEnabled() { slog.Info("devcontainer prebuild cache enabled", "repository", s.cfg.PrebuildRepository) } diff --git a/server/internal/workspace/manager.go b/server/internal/workspace/manager.go index bd696cb..805c840 100644 --- a/server/internal/workspace/manager.go +++ b/server/internal/workspace/manager.go @@ -70,6 +70,16 @@ type Manager struct { // (see prebuild.go). Empty keeps Create on its original path. prebuildRepo string + // vscodeCacheDir enables carrying ~/.vscode-server across container + // recreates when non-empty (see vscode_cache.go). + vscodeCacheDir string + + // resolveTargetHook is the seam the vscode-cache tests use to stand in + // for the container/user/home lookup, which would otherwise shell out + // to docker. Nil in production. Mirrors the per-instance hook pattern + // at sshproxy.Server.resolveContainerHook. + resolveTargetHook func(ctx context.Context, workspaceID string) (container, user, home string, err error) + // userMu guards userCache, which memoizes ContainerUser lookups. // VS Code Remote-SSH opens many channels per connection and each one // resolves the exec user, so an uncached `docker inspect` per channel diff --git a/server/internal/workspace/prebuild_e2e_test.go b/server/internal/workspace/prebuild_e2e_test.go index cc58c38..4f1217f 100644 --- a/server/internal/workspace/prebuild_e2e_test.go +++ b/server/internal/workspace/prebuild_e2e_test.go @@ -106,3 +106,100 @@ func TestEnsurePrebuild_EndToEnd(t *testing.T) { } } } + +// TestVSCodeServerCache_EndToEnd proves the docker cp round-trip: a payload +// written into a container survives the container being destroyed and +// recreated, and lands owned by the devcontainer's remoteUser. +// +// DEUCE_PREBUILD_E2E=1 go test ./internal/workspace/ -run VSCodeServerCache -v -timeout 20m +func TestVSCodeServerCache_EndToEnd(t *testing.T) { + if os.Getenv("DEUCE_PREBUILD_E2E") == "" { + t.Skip("set DEUCE_PREBUILD_E2E=1 to run (shells out to devpod + docker)") + } + for _, bin := range []string{"devpod", "docker"} { + if _, err := exec.LookPath(bin); err != nil { + t.Skipf("%s not on PATH", bin) + } + } + + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, ".devcontainer"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, ".devcontainer", "devcontainer.json"), + []byte(`{"name":"deuce-cache-e2e","image":"mcr.microsoft.com/devcontainers/base:ubuntu","remoteUser":"vscode"}`), 0o644); err != nil { + t.Fatal(err) + } + + m := NewManager("devpod", "docker", "") + m.SetVSCodeCacheDir(t.TempDir()) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Minute) + defer cancel() + + const wsID = "deuce-cache-e2e-ws" + t.Cleanup(func() { + _ = exec.Command("devpod", "delete", wsID, "--force", "--ignore-not-found").Run() + }) + + logFn := func(line string) { t.Log(line) } + if _, err := m.Create(ctx, wsID, dir, logFn); err != nil { + t.Fatalf("initial Create: %v", err) + } + + // Stand in for what VS Code Remote-SSH would install. + seed := `mkdir -p "$HOME/.vscode-server/bin/deadbeef" && echo cached-payload > "$HOME/.vscode-server/bin/deadbeef/marker"` + if out, err := m.ExecInWorkspace(ctx, wsID, seed).CombinedOutput(); err != nil { + t.Fatalf("seed payload: %v (%s)", err, out) + } + + if err := m.SaveVSCodeServer(ctx, wsID, logFn); err != nil { + t.Fatalf("SaveVSCodeServer: %v", err) + } + + // Destroy and recreate — the case that costs a ~120MB re-download today. + if err := m.Delete(ctx, wsID); err != nil { + t.Fatalf("Delete: %v", err) + } + if _, err := m.Create(ctx, wsID, dir, logFn); err != nil { + t.Fatalf("recreate: %v", err) + } + + // Gone from the fresh container... + if out, err := m.ExecInWorkspace(ctx, wsID, `cat "$HOME/.vscode-server/bin/deadbeef/marker" 2>/dev/null || echo ABSENT`).CombinedOutput(); err != nil { + t.Fatalf("pre-restore check: %v (%s)", err, out) + } else if !strings.Contains(string(out), "ABSENT") { + t.Fatalf("payload unexpectedly survived container recreate: %s", out) + } + + if err := m.RestoreVSCodeServer(ctx, wsID, logFn); err != nil { + t.Fatalf("RestoreVSCodeServer: %v", err) + } + + checks := map[string]string{ + "payload restored": `cat "$HOME/.vscode-server/bin/deadbeef/marker"`, + "owned by session": `stat -c %U "$HOME/.vscode-server"`, + "writable": `touch "$HOME/.vscode-server/writetest" && echo writable`, + } + want := map[string]string{ + "payload restored": "cached-payload", + "owned by session": "vscode", + "writable": "writable", + } + for name, cmd := range checks { + out, err := m.ExecInWorkspace(ctx, wsID, cmd).CombinedOutput() + got := strings.TrimSpace(string(out)) + if err != nil { + t.Errorf("%s: %v (output: %s)", name, err, got) + continue + } + t.Logf("%s -> %s", name, got) + if !strings.Contains(got, want[name]) { + t.Errorf("%s = %q, want it to contain %q", name, got, want[name]) + } + } + + if err := m.PurgeVSCodeServer(wsID); err != nil { + t.Errorf("PurgeVSCodeServer: %v", err) + } +} diff --git a/server/internal/workspace/vscode_cache.go b/server/internal/workspace/vscode_cache.go new file mode 100644 index 0000000..7215e2e --- /dev/null +++ b/server/internal/workspace/vscode_cache.go @@ -0,0 +1,233 @@ +package workspace + +import ( + "context" + "errors" + "fmt" + "log/slog" + "os" + "path/filepath" + "regexp" + "strings" +) + +// vscodeServerDirName is the directory VS Code Remote-SSH installs its server +// payload into, inside the container's home. Roughly 120MB, re-downloaded on +// every container recreate unless it is carried across. +const vscodeServerDirName = ".vscode-server" + +// validWorkspaceID bounds what may become a path segment under the cache +// root. Workspace IDs are Deuce-generated, so this is defence in depth +// against a future ID scheme rather than untrusted input — but it is what +// stops "../.." from escaping the cache directory. +var validWorkspaceID = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,254}$`) + +// validAbsPath rejects a home directory that does not look like one, so a +// surprising `getent` result cannot send `docker cp` somewhere unexpected. +var validAbsPath = regexp.MustCompile(`^/[a-zA-Z0-9_./-]*$`) + +// ErrVSCodeCacheDisabled is returned when no cache directory is configured. +var ErrVSCodeCacheDisabled = errors.New("vscode-server cache directory not configured") + +// SetVSCodeCacheDir turns on the ~/.vscode-server cache. Empty (the default) +// disables it, and every cache operation becomes a no-op. +func (m *Manager) SetVSCodeCacheDir(dir string) { + m.vscodeCacheDir = dir +} + +// VSCodeCacheEnabled reports whether a cache directory is configured. +func (m *Manager) VSCodeCacheEnabled() bool { + return m.vscodeCacheDir != "" +} + +// cachePathFor returns the host directory holding a workspace's cached +// ~/.vscode-server tree. +// +// The cache is keyed by workspace, not by user. A workspace's container is +// already shared by every member of its session, so a per-workspace cache +// adds no new reach. Keying it per user and restoring into a shared +// container would be a wider win but would copy one user's extension state +// — including whatever credentials their extensions have stored — into a +// container other session members hold a shell on. +func (m *Manager) cachePathFor(workspaceID string) (string, error) { + if m.vscodeCacheDir == "" { + return "", ErrVSCodeCacheDisabled + } + if !validWorkspaceID.MatchString(workspaceID) { + return "", fmt.Errorf("workspace id %q is not usable as a cache path segment", workspaceID) + } + return filepath.Join(m.vscodeCacheDir, workspaceID), nil +} + +// containerHome resolves the home directory of the user `docker exec` runs +// as, from the passwd database rather than $HOME — `docker exec` does not +// set $HOME, so reading it would yield root's home or nothing at all. +func (m *Manager) containerHome(ctx context.Context, container, user string) (string, error) { + args := []string{"exec"} + if user != "" { + args = append(args, "--user", user) + } + args = append(args, container, "sh", "-c", `getent passwd "$(id -un)" | cut -d: -f6`) + + out, err := m.runner(ctx, "docker", args...) + if err != nil { + return "", fmt.Errorf("resolve container home: %w", err) + } + home := strings.TrimSpace(string(out)) + if home == "" || !validAbsPath.MatchString(home) { + return "", fmt.Errorf("container returned an implausible home directory %q", home) + } + return home, nil +} + +// resolveContainerTarget resolves the container name, its exec user and that +// user's home in one go — the three things every cache operation needs. +func (m *Manager) resolveContainerTarget(ctx context.Context, workspaceID string) (container, user, home string, err error) { + if m.resolveTargetHook != nil { + return m.resolveTargetHook(ctx, workspaceID) + } + container, err = m.ContainerName(ctx, workspaceID) + if err != nil { + return "", "", "", err + } + user, err = m.ContainerUser(ctx, container) + if err != nil { + // Not fatal on its own: an empty user means "leave the image's + // USER alone", which is the same fallback docker exec already uses. + slog.Debug("could not resolve container user for vscode cache", "workspace", workspaceID, "error", err) + user = "" + } + home, err = m.containerHome(ctx, container, user) + if err != nil { + return "", "", "", err + } + return container, user, home, nil +} + +// SaveVSCodeServer copies the container's ~/.vscode-server tree out to the +// host cache so the next container for this workspace can start from it. +// +// Call it before an action that destroys the container (stop, rebuild). A +// missing directory is not an error — it just means VS Code was never opened +// against this workspace. +func (m *Manager) SaveVSCodeServer(ctx context.Context, workspaceID string, logFn LogFunc) error { + dst, err := m.cachePathFor(workspaceID) + if err != nil { + return err + } + + // The user is only needed to resolve home; docker cp itself runs as root. + container, _, home, err := m.resolveContainerTarget(ctx, workspaceID) + if err != nil { + return err + } + + src := container + ":" + filepath.Join(home, vscodeServerDirName) + + // Stage into a sibling directory and swap, so an interrupted copy cannot + // leave a half-written tree that a later restore would push into a + // container as if it were complete. + staging := dst + ".partial" + if err := os.RemoveAll(staging); err != nil { + return fmt.Errorf("clear staging dir: %w", err) + } + if err := os.MkdirAll(staging, 0o700); err != nil { + return fmt.Errorf("create staging dir: %w", err) + } + + if out, err := m.runner(ctx, "docker", "cp", "-a", src, staging); err != nil { + _ = os.RemoveAll(staging) + // The overwhelmingly common case: VS Code was never opened here. + if strings.Contains(string(out), "No such container:path") || strings.Contains(string(out), "not found") { + slog.Debug("no vscode-server directory to cache", "workspace", workspaceID) + return nil + } + return fmt.Errorf("docker cp out: %w: %s", err, strings.TrimSpace(string(out))) + } + + if err := os.RemoveAll(dst); err != nil { + _ = os.RemoveAll(staging) + return fmt.Errorf("clear cache dir: %w", err) + } + if err := os.Rename(staging, dst); err != nil { + _ = os.RemoveAll(staging) + return fmt.Errorf("promote staging dir: %w", err) + } + + slog.Info("cached vscode-server payload", "workspace", workspaceID, "path", dst) + if logFn != nil { + logFn("Saved the VS Code server payload for reuse on the next container") + } + return nil +} + +// RestoreVSCodeServer copies a previously cached ~/.vscode-server tree back +// into a freshly created container, so VS Code Remote-SSH finds its server +// already installed instead of downloading it again. +// +// A cache miss is not an error — the first session for a workspace has +// nothing to restore. +func (m *Manager) RestoreVSCodeServer(ctx context.Context, workspaceID string, logFn LogFunc) error { + src, err := m.cachePathFor(workspaceID) + if err != nil { + return err + } + payload := filepath.Join(src, vscodeServerDirName) + if _, statErr := os.Stat(payload); statErr != nil { + if errors.Is(statErr, os.ErrNotExist) { + return nil + } + return fmt.Errorf("stat cached payload: %w", statErr) + } + + container, user, home, err := m.resolveContainerTarget(ctx, workspaceID) + if err != nil { + return err + } + + // Copy the directory itself into the home, reproducing the original + // layout. -a preserves the uids recorded when it was copied out, which + // are this same container user's. + if out, err := m.runner(ctx, "docker", "cp", "-a", payload, container+":"+home); err != nil { + return fmt.Errorf("docker cp in: %w: %s", err, strings.TrimSpace(string(out))) + } + + // Belt and braces: if the image's uid numbering changed between the two + // containers, the restored tree would land unwritable for the session + // user and VS Code would fail in a confusing way rather than re-download. + if user != "" { + target := filepath.Join(home, vscodeServerDirName) + if out, chErr := m.runner(ctx, "docker", "exec", "--user", "root", container, + "chown", "-R", user+":"+user, target); chErr != nil { + slog.Warn("could not chown restored vscode-server payload", + "workspace", workspaceID, "error", chErr, "output", strings.TrimSpace(string(out))) + } + } + + slog.Info("restored vscode-server payload from cache", "workspace", workspaceID) + if logFn != nil { + logFn("Restored the cached VS Code server payload — no re-download needed") + } + return nil +} + +// PurgeVSCodeServer removes a workspace's cached payload. Called when the +// workspace is deleted, so caches do not outlive what they belong to — the +// retention policy the plan's System-Wide Impact section asks for. +func (m *Manager) PurgeVSCodeServer(workspaceID string) error { + dir, err := m.cachePathFor(workspaceID) + if err != nil { + if errors.Is(err, ErrVSCodeCacheDisabled) { + return nil + } + return err + } + if err := os.RemoveAll(dir); err != nil { + return fmt.Errorf("purge vscode-server cache: %w", err) + } + if err := os.RemoveAll(dir + ".partial"); err != nil { + return fmt.Errorf("purge vscode-server staging: %w", err) + } + slog.Info("purged vscode-server cache", "workspace", workspaceID) + return nil +} diff --git a/server/internal/workspace/vscode_cache_test.go b/server/internal/workspace/vscode_cache_test.go new file mode 100644 index 0000000..8450366 --- /dev/null +++ b/server/internal/workspace/vscode_cache_test.go @@ -0,0 +1,197 @@ +package workspace + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestVSCodeCacheEnabled(t *testing.T) { + m := NewManager("devpod", "docker", "") + if m.VSCodeCacheEnabled() { + t.Error("cache should be off by default") + } + m.SetVSCodeCacheDir("/var/lib/deuce/vscode") + if !m.VSCodeCacheEnabled() { + t.Error("cache should be on once a directory is configured") + } +} + +func TestCachePathFor(t *testing.T) { + m := NewManager("devpod", "docker", "") + + if _, err := m.cachePathFor("ws1"); !errors.Is(err, ErrVSCodeCacheDisabled) { + t.Errorf("expected ErrVSCodeCacheDisabled when unconfigured, got %v", err) + } + + m.SetVSCodeCacheDir("/var/lib/deuce/vscode") + got, err := m.cachePathFor("ws1") + if err != nil { + t.Fatal(err) + } + if want := "/var/lib/deuce/vscode/ws1"; got != want { + t.Errorf("cachePathFor = %q, want %q", got, want) + } +} + +// TestCachePathFor_RejectsTraversal is the containment guarantee: a +// workspace id must never be able to steer writes outside the cache root. +func TestCachePathFor_RejectsTraversal(t *testing.T) { + m := NewManager("devpod", "docker", "") + m.SetVSCodeCacheDir("/var/lib/deuce/vscode") + + hostile := []string{ + "../../etc", + "..", + "/absolute", + "ws/../../escape", + "", + "with space", + "semi;colon", + } + for _, id := range hostile { + if got, err := m.cachePathFor(id); err == nil { + t.Errorf("workspace id %q should be rejected, got path %q", id, got) + } + } +} + +// TestContainerHome_ResolvesFromPasswd covers why $HOME is not read directly: +// `docker exec` does not set it, so the passwd database is the only reliable +// source for where ~/.vscode-server lives. +func TestContainerHome_ResolvesFromPasswd(t *testing.T) { + m := NewManager("devpod", "docker", "") + var gotArgs []string + m.runner = func(_ context.Context, name string, args ...string) ([]byte, error) { + gotArgs = append([]string{name}, args...) + return []byte("/home/vscode\n"), nil + } + + home, err := m.containerHome(context.Background(), "devpod-abc", "vscode") + if err != nil { + t.Fatal(err) + } + if home != "/home/vscode" { + t.Errorf("home = %q, want /home/vscode", home) + } + joined := strings.Join(gotArgs, " ") + if !strings.Contains(joined, "--user vscode") { + t.Errorf("expected the exec to run as the container user: %v", gotArgs) + } + if !strings.Contains(joined, "getent passwd") { + t.Errorf("expected a passwd lookup rather than $HOME: %v", gotArgs) + } +} + +func TestContainerHome_RejectsImplausibleValues(t *testing.T) { + m := NewManager("devpod", "docker", "") + for _, out := range []string{"", " ", "not-absolute", "/home/$(hostile)", "/home/a b"} { + m.runner = func(_ context.Context, _ string, _ ...string) ([]byte, error) { + return []byte(out + "\n"), nil + } + if home, err := m.containerHome(context.Background(), "devpod-abc", "vscode"); err == nil { + t.Errorf("output %q should be rejected, got home %q", out, home) + } + } +} + +func TestContainerHome_OmitsUserFlagWhenUnknown(t *testing.T) { + m := NewManager("devpod", "docker", "") + var gotArgs []string + m.runner = func(_ context.Context, name string, args ...string) ([]byte, error) { + gotArgs = append([]string{name}, args...) + return []byte("/root\n"), nil + } + if _, err := m.containerHome(context.Background(), "devpod-abc", ""); err != nil { + t.Fatal(err) + } + if strings.Contains(strings.Join(gotArgs, " "), "--user") { + t.Errorf("empty user must not produce a --user flag: %v", gotArgs) + } +} + +// TestRestoreVSCodeServer_CacheMissIsNotAnError covers the first-session +// path: nothing cached yet must be silent, not a logged failure. +func TestRestoreVSCodeServer_CacheMissIsNotAnError(t *testing.T) { + m := NewManager("devpod", "docker", "") + m.SetVSCodeCacheDir(t.TempDir()) + m.runner = func(_ context.Context, _ string, _ ...string) ([]byte, error) { + t.Error("restore must not touch docker when there is no cached payload") + return nil, nil + } + if err := m.RestoreVSCodeServer(context.Background(), "ws1", nil); err != nil { + t.Errorf("cache miss should be a no-op, got %v", err) + } +} + +// TestPurgeVSCodeServer removes both the promoted cache and any staging +// directory left by an interrupted save. +func TestPurgeVSCodeServer(t *testing.T) { + root := t.TempDir() + m := NewManager("devpod", "docker", "") + m.SetVSCodeCacheDir(root) + + for _, dir := range []string{"ws1", "ws1.partial"} { + if err := os.MkdirAll(filepath.Join(root, dir, vscodeServerDirName), 0o700); err != nil { + t.Fatal(err) + } + } + + if err := m.PurgeVSCodeServer("ws1"); err != nil { + t.Fatal(err) + } + for _, dir := range []string{"ws1", "ws1.partial"} { + if _, err := os.Stat(filepath.Join(root, dir)); !errors.Is(err, os.ErrNotExist) { + t.Errorf("%s should be gone after purge, stat err = %v", dir, err) + } + } +} + +func TestPurgeVSCodeServer_DisabledIsNoOp(t *testing.T) { + m := NewManager("devpod", "docker", "") + if err := m.PurgeVSCodeServer("ws1"); err != nil { + t.Errorf("purge with the cache disabled should be a no-op, got %v", err) + } +} + +// TestSaveVSCodeServer_PromotesAtomically checks the staging swap: a +// successful save must leave the payload at the final path and no .partial +// directory behind, so a later restore cannot pick up a half-copied tree. +func TestSaveVSCodeServer_PromotesAtomically(t *testing.T) { + root := t.TempDir() + m := NewManager("devpod", "docker", "") + m.SetVSCodeCacheDir(root) + + m.runner = func(_ context.Context, _ string, args ...string) ([]byte, error) { + switch { + case args[0] == "exec": + return []byte("/home/vscode\n"), nil + case args[0] == "cp": + // args: cp -a + dst := args[len(args)-1] + return nil, os.MkdirAll(filepath.Join(dst, vscodeServerDirName), 0o700) + case args[0] == "image": + return []byte("[]"), nil + } + return []byte("[]"), nil + } + // ContainerName/ContainerUser would shell out; short-circuit by seeding + // the user cache and stubbing the container lookup through the runner. + m.resolveTargetHook = func(context.Context, string) (string, string, string, error) { + return "devpod-abc", "vscode", "/home/vscode", nil + } + + if err := m.SaveVSCodeServer(context.Background(), "ws1", nil); err != nil { + t.Fatal(err) + } + + if _, err := os.Stat(filepath.Join(root, "ws1", vscodeServerDirName)); err != nil { + t.Errorf("payload should be promoted to the final path: %v", err) + } + if _, err := os.Stat(filepath.Join(root, "ws1.partial")); !errors.Is(err, os.ErrNotExist) { + t.Errorf("staging directory should be gone after promotion, stat err = %v", err) + } +} From 0cf64001fe5e488fa0a4d9e0764bf47fd314684a Mon Sep 17 00:00:00 2001 From: Clint Berry Date: Tue, 28 Jul 2026 21:42:02 +0000 Subject: [PATCH 3/4] fix(workspace): detect docker's actual missing-path wording when caching docker reports an absent source path as "Could not find the file in container ". The check matched "not found", which that string does not contain, so a workspace that had never been opened in VS Code logged a save failure on every stop and rebuild. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014orMLVuaREojZaJ6mzXvtv --- server/internal/workspace/vscode_cache.go | 26 +++++++++++++-- .../internal/workspace/vscode_cache_test.go | 33 +++++++++++++++++-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/server/internal/workspace/vscode_cache.go b/server/internal/workspace/vscode_cache.go index 7215e2e..60a032c 100644 --- a/server/internal/workspace/vscode_cache.go +++ b/server/internal/workspace/vscode_cache.go @@ -127,6 +127,11 @@ func (m *Manager) SaveVSCodeServer(ctx context.Context, workspaceID string, logF // Stage into a sibling directory and swap, so an interrupted copy cannot // leave a half-written tree that a later restore would push into a // container as if it were complete. + // + // Two concurrent saves for the same workspace would contend over the + // staging directory, but the worst case is that one of them fails and + // logs: the promoted cache is only ever replaced by a completed rename, + // never by a partial copy. staging := dst + ".partial" if err := os.RemoveAll(staging); err != nil { return fmt.Errorf("clear staging dir: %w", err) @@ -137,8 +142,10 @@ func (m *Manager) SaveVSCodeServer(ctx context.Context, workspaceID string, logF if out, err := m.runner(ctx, "docker", "cp", "-a", src, staging); err != nil { _ = os.RemoveAll(staging) - // The overwhelmingly common case: VS Code was never opened here. - if strings.Contains(string(out), "No such container:path") || strings.Contains(string(out), "not found") { + // The overwhelmingly common case: VS Code was never opened against + // this workspace, so there is simply nothing to cache. Treating it + // as an error would warn on every stop of every such workspace. + if isMissingContainerPath(string(out)) { slog.Debug("no vscode-server directory to cache", "workspace", workspaceID) return nil } @@ -161,6 +168,21 @@ func (m *Manager) SaveVSCodeServer(ctx context.Context, workspaceID string, logF return nil } +// isMissingContainerPath reports whether a `docker cp` failure was just an +// absent source path. Docker phrases this as +// +// Error response from daemon: Could not find the file /home/vscode/.vscode-server in container +// +// Note "Could not find" — matching on "not found" silently misses it, which +// is how this turned into a warning on every stop of a workspace that had +// never been opened in VS Code. +func isMissingContainerPath(dockerOutput string) bool { + s := strings.ToLower(dockerOutput) + return strings.Contains(s, "could not find the file") || + strings.Contains(s, "no such container:path") || + strings.Contains(s, "not found in container") +} + // RestoreVSCodeServer copies a previously cached ~/.vscode-server tree back // into a freshly created container, so VS Code Remote-SSH finds its server // already installed instead of downloading it again. diff --git a/server/internal/workspace/vscode_cache_test.go b/server/internal/workspace/vscode_cache_test.go index 8450366..796a22c 100644 --- a/server/internal/workspace/vscode_cache_test.go +++ b/server/internal/workspace/vscode_cache_test.go @@ -178,8 +178,8 @@ func TestSaveVSCodeServer_PromotesAtomically(t *testing.T) { } return []byte("[]"), nil } - // ContainerName/ContainerUser would shell out; short-circuit by seeding - // the user cache and stubbing the container lookup through the runner. + // ContainerName/ContainerUser would shell out to docker; the hook stands + // in for the whole container/user/home resolution. m.resolveTargetHook = func(context.Context, string) (string, string, string, error) { return "devpod-abc", "vscode", "/home/vscode", nil } @@ -195,3 +195,32 @@ func TestSaveVSCodeServer_PromotesAtomically(t *testing.T) { t.Errorf("staging directory should be gone after promotion, stat err = %v", err) } } + +// TestIsMissingContainerPath pins the detection against docker's real +// wording. "Could not find the file" does not contain "not found", so a +// naive match reported every never-opened workspace as a save failure. +func TestIsMissingContainerPath(t *testing.T) { + missing := []string{ + "Error response from daemon: Could not find the file /home/vscode/.vscode-server in container ca466b78eedf", + "Error: No such container:path: devpod-abc:/home/vscode/.vscode-server", + "lstat /home/vscode/.vscode-server: not found in container", + } + for _, out := range missing { + if !isMissingContainerPath(out) { + t.Errorf("should be treated as an absent path: %q", out) + } + } + + // Real failures must stay real — they need the error path, not silence. + genuine := []string{ + "Error response from daemon: container ca466b78eedf is not running", + "permission denied", + "Cannot connect to the Docker daemon", + "", + } + for _, out := range genuine { + if isMissingContainerPath(out) { + t.Errorf("should NOT be swallowed as an absent path: %q", out) + } + } +} From 2f40e7ae6e30d17090943bbf3db52b875a0da7fc Mon Sep 17 00:00:00 2001 From: Clint Berry Date: Thu, 30 Jul 2026 20:37:45 +0000 Subject: [PATCH 4/4] docs(env): document the two new cache settings in .env.example Both were documented in CLAUDE.md but missing from the example env file, which is where an operator configuring a deployment actually looks. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014orMLVuaREojZaJ6mzXvtv --- server/.env.example | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/server/.env.example b/server/.env.example index 87890ae..3d4654e 100644 --- a/server/.env.example +++ b/server/.env.example @@ -14,5 +14,19 @@ GITHUB_TOKEN= DEVPOD_BIN=devpod DEVPOD_PROVIDER=docker +# Devcontainer prebuild cache. Empty (default) = off: every session builds the +# devcontainer from scratch and installs Pi over `devpod ssh`. Set it to a +# Docker repository name (no tag — the devcontainer hash is appended) to build +# each repo's image once with Pi and the agent tooling baked in, so later +# sessions start with no build and no install. Stays local to the Docker +# daemon; no registry or push is involved. +DEUCE_PREBUILD_REPOSITORY= + +# Carries a workspace's ~/.vscode-server tree across container recreates so +# VS Code Remote-SSH does not re-download its ~120MB server payload each time. +# Empty (default) = off. Must be an absolute path. Budget ~120MB per workspace +# opened in VS Code; entries are removed when their workspace is deleted. +DEUCE_VSCODE_SERVER_CACHE_DIR= + # Anthropic API key for real agent execution ANTHROPIC_API_KEY=