From ea8b30e1b25092b24ebc05ea20341caa60e61c39 Mon Sep 17 00:00:00 2001 From: fullsend-code Date: Thu, 28 May 2026 08:25:15 +0000 Subject: [PATCH] fix(#1649): add UploadFile for file-to-file sandbox upload semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit openshell sandbox upload treats the remote path as a directory, placing the uploaded file inside it with the source basename. This caused host_files staging to create a directory at the destination path (e.g. prior-review.txt/) with the file nested inside, producing EISDIR errors when the agent tried to read the file. Add sandbox.UploadFile() — analogous to the existing DownloadFile() — which uploads to the parent directory and renames in-sandbox if the basename differs. Update all three sandbox.Upload() calls in bootstrapEnv's host_files staging and .env upload to use UploadFile instead. Note: Go tests could not run in sandbox (go.mod requires go >= 1.26.0, sandbox has 1.24.13 with read-only GOPATH). Pre-commit could not run (same Go toolchain issue). Manual verification of go-test is required. Closes #1649 --- internal/cli/run.go | 6 +++--- internal/sandbox/sandbox.go | 29 +++++++++++++++++++++++++++++ internal/sandbox/sandbox_test.go | 11 +++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/internal/cli/run.go b/internal/cli/run.go index d621a00d87..7f6a0d460e 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -957,7 +957,7 @@ func bootstrapEnv(sandboxName, repoDir string, h *harness.Harness) error { } tmpFile.Close() - if err := sandbox.Upload(sandboxName, tmpFile.Name(), remoteEnvFile); err != nil { + if err := sandbox.UploadFile(sandboxName, tmpFile.Name(), remoteEnvFile); err != nil { return fmt.Errorf("copying .env file to sandbox: %w", err) } @@ -998,13 +998,13 @@ func bootstrapEnv(sandboxName, repoDir string, h *harness.Harness) error { } tmp.Close() - if err := sandbox.Upload(sandboxName, tmp.Name(), hf.Dest); err != nil { + if err := sandbox.UploadFile(sandboxName, tmp.Name(), hf.Dest); err != nil { os.Remove(tmp.Name()) return fmt.Errorf("copying expanded file %s to %s: %w", hf.Src, hf.Dest, err) } os.Remove(tmp.Name()) } else { - if err := sandbox.Upload(sandboxName, hostPath, hf.Dest); err != nil { + if err := sandbox.UploadFile(sandboxName, hostPath, hf.Dest); err != nil { return fmt.Errorf("copying host file %s to %s: %w", hf.Src, hf.Dest, err) } } diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go index 1df64674af..b24f8a3b2c 100644 --- a/internal/sandbox/sandbox.go +++ b/internal/sandbox/sandbox.go @@ -377,6 +377,35 @@ func Upload(sandboxName, localPath, remotePath string) error { return nil } +// UploadFile copies a single local file into a sandbox at a specific remote path. +// openshell sandbox upload treats the remote path as a directory (similar to +// docker cp), placing the file inside it with the source basename. This function +// uploads to the parent directory and renames inside the sandbox if the resulting +// filename differs from the desired remote name, ensuring file-to-file semantics. +// This mirrors the DownloadFile approach. +func UploadFile(sandboxName, localPath, remotePath string) error { + remoteDir := filepath.Dir(remotePath) + + if err := Upload(sandboxName, localPath, remoteDir); err != nil { + return err + } + + // openshell places the file as /. + // If that differs from the desired remotePath, rename it in-sandbox. + uploadedPath := filepath.Join(remoteDir, filepath.Base(localPath)) + if uploadedPath != remotePath { + mvCmd := fmt.Sprintf("mv %s %s", uploadedPath, remotePath) + _, stderr, exitCode, err := Exec(sandboxName, mvCmd, 10*time.Second) + if err != nil { + return fmt.Errorf("renaming uploaded file in sandbox %q: %w", sandboxName, err) + } + if exitCode != 0 { + return fmt.Errorf("renaming uploaded file in sandbox %q: exit %d: %s", sandboxName, exitCode, stderr) + } + } + return nil +} + // UploadDir uploads a local directory into a sandbox, preserving symlinks. // openshell sandbox upload dereferences symlinks; this builds a local tarball // with --no-dereference, uploads it, and extracts it in the sandbox. diff --git a/internal/sandbox/sandbox_test.go b/internal/sandbox/sandbox_test.go index 990fa49336..c0792bd08d 100644 --- a/internal/sandbox/sandbox_test.go +++ b/internal/sandbox/sandbox_test.go @@ -378,6 +378,17 @@ func TestEffectiveReadyTimeout_EnvVarCappedAtMax(t *testing.T) { assert.Equal(t, maxReadyTimeout, got) } +func TestUploadFile_OpenshellNotInPath(t *testing.T) { + tmp := t.TempDir() + f := filepath.Join(tmp, "test.txt") + require.NoError(t, os.WriteFile(f, []byte("hello"), 0o644)) + + t.Setenv("PATH", "") + + err := UploadFile("test-sandbox", f, "/tmp/workspace/test.txt") + assert.Error(t, err) +} + func TestUploadDir_OpenshellNotInPath(t *testing.T) { dir := t.TempDir() t.Setenv("PATH", "")