fix(sdk): make the git namespace usable from a caller's point of view

.changeset/box-git-fixes.md

@@ -0,0 +1,11 @@
+---
+"@upstash/box": patch
+---
+
+Fix a lost exit status in `exec.stream()`, and three things in the git namespace that made it unusable from a caller's point of view.
+
+- `exec.stream()` could end without an `exit` chunk. The `event: exit` marker and its `data:` payload can arrive in separate network reads; the parser matched a half-arrived line and returned, ending the stream with no exit status at all. A caller had no way to tell whether the command succeeded, and about one run in thirty of a short command was affected. The payload is now waited for.
+
+- `git.updateConfig()` sent its request to `/v2/box/{id}/git-config`, which the coordinator does not serve. The identity endpoint is `/v2/box/{id}/config/git`, so every call returned 404 and no git identity was ever set through the SDK.
+- `git.exec()` results now carry `exit_code`. The API has always returned it; the type omitted it, so callers could not tell a failed git command (for example exit 128 when the folder is not a repository) from a successful one.
+- `git.clone()` accepts `folder`, naming the directory the repository is cloned into. Unlike every other git operation, where the folder is an existing directory derived from `cd()`, clone's folder is the destination and does not exist yet, so it could not be expressed at all: `cd()` fails on a directory the clone is about to create.

packages/python-sdk/CHANGELOG.md

@@ -2,6 +2,17 @@
 
 All notable changes to `upstash-box` (Python) are documented here.
 
+## 0.3.1
+
+- Fix `git.update_config()` sending its request to `/v2/box/{id}/git-config`,
+  which the coordinator does not serve. The identity endpoint is
+  `/v2/box/{id}/config/git`, so every call returned 404 and no git identity was
+  ever set through the SDK.
+- Add `folder` to `git.clone()`, naming the directory the repository is cloned
+  into. Unlike every other git operation, where the folder is an existing
+  directory derived from `cd()`, clone's folder is the destination and does not
+  exist yet, so it could not be expressed at all.
+
 ## 0.3.0
 
 - `exec.session(...)` — live command sessions over a WebSocket, matching

packages/python-sdk/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "upstash-box"
-version = "0.3.0"
+version = "0.3.1"
 description = "Upstash Box SDK - Python client for async and parallel AI coding agents"
 readme = "README.md"
 license = { text = "MIT" }

packages/python-sdk/tests/_async/test_box_git.py

@@ -32,6 +32,17 @@ async def test_clone_with_depth():
     await box.aclose()
 
 
+@respx.mock
+async def test_clone_into_explicit_folder():
+    box = await make_async_box(respx.mock)
+    route = respx.post(f"{BASE}/git/clone").mock(return_value=httpx.Response(200, json={}))
+    # For clone the folder is the destination, so it does not exist yet and
+    # cd() cannot be used to express it.
+    await box.git.clone(repo="https://github.com/u/r", folder="my-app")
+    assert last_json_body(route)["folder"] == "my-app"
+    await box.aclose()
+
+
 @respx.mock
 async def test_diff_and_status():
     box = await make_async_box(respx.mock)
@@ -69,7 +80,9 @@ async def test_update_config_requires_one_field():
 @respx.mock
 async def test_update_config():
     box = await make_async_box(respx.mock)
-    route = respx.put(f"{BASE}/git-config").mock(
+    # The coordinator serves this under config/git; the old spelling is what
+    # let the wrong path ship in both SDKs.
+    route = respx.put(f"{BASE}/config/git").mock(
         return_value=httpx.Response(
             200, json={"git_user_name": "Jane", "git_user_email": "j@e.com"}
         )

packages/python-sdk/upstash_box/_async/client.py

@@ -396,9 +396,14 @@ def __init__(self, box: "AsyncBox") -> None:
         self._box = box
 
     async def clone(
-        self, *, repo: str, branch: Optional[str] = None, depth: Optional[int] = None
+        self,
+        *,
+        repo: str,
+        branch: Optional[str] = None,
+        depth: Optional[int] = None,
+        folder: Optional[str] = None,
     ) -> None:
-        await self._box._git_clone(repo, branch, depth)
+        await self._box._git_clone(repo, branch, depth, folder)
 
     async def diff(self) -> str:
         return await self._box._git_diff()
@@ -1656,8 +1661,10 @@ async def _schedule_delete(self, id: str) -> None:
 
     # ==================== Git ====================
 
-    async def _git_clone(self, repo, branch, depth) -> None:
-        folder = self._get_folder()
+    async def _git_clone(self, repo, branch, depth, folder=None) -> None:
+        # For clone the folder is the destination, so an explicit one wins over
+        # the current directory; the directory does not exist yet by definition.
+        folder = folder or self._get_folder()
         body: Dict[str, Any] = {"repo": repo, "branch": branch, "github_token": self._git_token}
         if depth is not None:
             body["depth"] = depth
@@ -1694,7 +1701,7 @@ async def _git_update_config(self, user_name, user_email) -> GitConfigResult:
             raise BoxError("At least one of user_name or user_email is required")
         data = await self._request(
             "PUT",
-            f"/v2/box/{self.id}/git-config",
+            f"/v2/box/{self.id}/config/git",
             body={"git_user_name": user_name, "git_user_email": user_email},
         )
         return GitConfigResult.model_validate(data)

packages/python-sdk/upstash_box/_sync/client.py

@@ -391,9 +391,14 @@ def __init__(self, box: "Box") -> None:
         self._box = box
 
     def clone(
-        self, *, repo: str, branch: Optional[str] = None, depth: Optional[int] = None
+        self,
+        *,
+        repo: str,
+        branch: Optional[str] = None,
+        depth: Optional[int] = None,
+        folder: Optional[str] = None,
     ) -> None:
-        self._box._git_clone(repo, branch, depth)
+        self._box._git_clone(repo, branch, depth, folder)
 
     def diff(self) -> str:
         return self._box._git_diff()
@@ -1641,8 +1646,10 @@ def _schedule_delete(self, id: str) -> None:
 
     # ==================== Git ====================
 
-    def _git_clone(self, repo, branch, depth) -> None:
-        folder = self._get_folder()
+    def _git_clone(self, repo, branch, depth, folder=None) -> None:
+        # For clone the folder is the destination, so an explicit one wins over
+        # the current directory; the directory does not exist yet by definition.
+        folder = folder or self._get_folder()
         body: Dict[str, Any] = {"repo": repo, "branch": branch, "github_token": self._git_token}
         if depth is not None:
             body["depth"] = depth
@@ -1679,7 +1686,7 @@ def _git_update_config(self, user_name, user_email) -> GitConfigResult:
             raise BoxError("At least one of user_name or user_email is required")
         data = self._request(
             "PUT",
-            f"/v2/box/{self.id}/git-config",
+            f"/v2/box/{self.id}/config/git",
             body={"git_user_name": user_name, "git_user_email": user_email},
         )
         return GitConfigResult.model_validate(data)

packages/sdk/src/__tests__/box-exec-stream.test.ts

@@ -38,6 +38,38 @@ function mockExecStreamResponse(
   } as Response;
 }
 
+/**
+ * A stream delivered in caller-chosen pieces, so a marker and its payload can
+ * be split the way the network splits them.
+ */
+function mockChunkedResponse(pieces: string[]): Response {
+  const encoder = new TextEncoder();
+  const stream = new ReadableStream({
+    start(controller) {
+      for (const piece of pieces) controller.enqueue(encoder.encode(piece));
+      controller.close();
+    },
+  });
+  return {
+    ok: true,
+    status: 200,
+    statusText: "OK",
+    headers: new Headers({ "content-type": "text/event-stream" }),
+    json: () => Promise.reject(new Error("stream response")),
+    text: () => Promise.resolve(pieces.join("")),
+    body: stream,
+    bodyUsed: false,
+    arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)),
+    blob: () => Promise.resolve(new Blob()),
+    formData: () => Promise.resolve(new FormData()),
+    clone: () => mockChunkedResponse(pieces),
+    redirected: false,
+    type: "basic" as ResponseType,
+    url: "",
+    bytes: () => Promise.resolve(new Uint8Array()),
+  } as Response;
+}
+
 function mockExecStreamErrorResponse(errorMessage: string): Response {
   const raw = `event: error\ndata: ${JSON.stringify({ error: errorMessage })}\n\n`;
   const encoder = new TextEncoder();
@@ -381,3 +413,55 @@ describe("exec.streamCode", () => {
     expect(run.cost.computeMs).toBeGreaterThanOrEqual(0);
   });
 });
+
+describe("exec stream split across network reads", () => {
+  async function collect(box: Awaited<ReturnType<typeof createTestBox>>["box"]) {
+    const chunks: ExecStreamChunk[] = [];
+    for await (const chunk of await box.exec.stream("true")) chunks.push(chunk);
+    return chunks;
+  }
+
+  it("still reports the exit status when the marker and payload arrive apart", async () => {
+    const { box, fetchMock } = await createTestBox();
+    // The marker completing one read and the payload starting the next is what
+    // happens on the wire; losing the exit chunk there leaves a caller with no
+    // way to know whether the command succeeded.
+    fetchMock.mockResolvedValueOnce(
+      mockChunkedResponse(["hello\n", "event: exit\n", 'data: {"exit_code":3,"cpu_ns":1}\n\n']),
+    );
+
+    const chunks = await collect(box);
+
+    expect(chunks.filter((chunk) => chunk.type === "exit")).toEqual([
+      { type: "exit", exitCode: 3, cpuNs: 1 },
+    ]);
+  });
+
+  it("does not duplicate the output that preceded the exit event", async () => {
+    const { box, fetchMock } = await createTestBox();
+    fetchMock.mockResolvedValueOnce(
+      mockChunkedResponse(["hello\n", "event: exit\n", 'data: {"exit_code":0,"cpu_ns":0}\n\n']),
+    );
+
+    const chunks = await collect(box);
+
+    const output = chunks
+      .filter((chunk) => chunk.type === "output")
+      .map((chunk) => (chunk as { data: string }).data)
+      .join("");
+    expect(output).toBe("hello\n");
+  });
+
+  it("survives the payload itself being split", async () => {
+    const { box, fetchMock } = await createTestBox();
+    fetchMock.mockResolvedValueOnce(
+      mockChunkedResponse(["event: exit\n", 'data: {"exit_code":', '42,"cpu_ns":0}\n\n']),
+    );
+
+    const chunks = await collect(box);
+
+    expect(chunks.filter((chunk) => chunk.type === "exit")).toEqual([
+      { type: "exit", exitCode: 42, cpuNs: 0 },
+    ]);
+  });
+});

packages/sdk/src/__tests__/box-git.test.ts

@@ -27,6 +27,30 @@ describe("Box git operations", () => {
       expect(body.branch).toBe("dev");
     });
 
+    it("clones into an explicit destination folder", async () => {
+      const { box, fetchMock } = await createTestBox();
+      fetchMock.mockResolvedValueOnce(mockResponse({}));
+
+      // For clone the folder is where the repo lands, so it does not exist yet
+      // and cd() cannot be used to express it.
+      await box.git.clone({ repo: "owner/repo", folder: "my-app" });
+
+      const body = JSON.parse(fetchMock.mock.calls[1]![1]?.body as string);
+      expect(body.folder).toBe("my-app");
+    });
+
+    it("prefers the explicit destination over the current directory", async () => {
+      const { box, fetchMock } = await createTestBox();
+      fetchMock.mockResolvedValueOnce(mockResponse({ exit_code: 0, output: "" }));
+      await box.cd("/workspace/home/elsewhere");
+      fetchMock.mockResolvedValueOnce(mockResponse({}));
+
+      await box.git.clone({ repo: "owner/repo", folder: "my-app" });
+
+      const body = JSON.parse(fetchMock.mock.calls.at(-1)![1]?.body as string);
+      expect(body.folder).toBe("my-app");
+    });
+
     it("clones a repo with depth", async () => {
       const { box, fetchMock } = await createTestBox();
       fetchMock.mockResolvedValueOnce(mockResponse({}));
@@ -113,7 +137,9 @@ describe("Box git operations", () => {
       });
 
       const [url, init] = fetchMock.mock.calls[1]!;
-      expect(url).toContain("/git-config");
+      // The coordinator serves this under config/git; asserting the old
+      // "/git-config" spelling is what let the wrong path ship.
+      expect(url).toContain("/config/git");
       expect(init?.method).toBe("PUT");
       const body = JSON.parse(init?.body as string);
       expect(body.git_user_name).toBe("John Doe");
@@ -174,16 +200,26 @@ describe("Box git operations", () => {
   describe("git.exec", () => {
     it("executes a git command", async () => {
       const { box, fetchMock } = await createTestBox();
-      fetchMock.mockResolvedValueOnce(mockResponse({ output: "abc123\ndef456" }));
+      fetchMock.mockResolvedValueOnce(mockResponse({ output: "abc123\ndef456", exit_code: 0 }));
 
       const result = await box.git.exec({ args: ["log", "--oneline", "-2"] });
       expect(result.output).toBe("abc123\ndef456");
+      expect(result.exit_code).toBe(0);
 
       const [url, init] = fetchMock.mock.calls[1]!;
       expect(url).toContain("/git/exec");
       const body = JSON.parse(init?.body as string);
       expect(body.args).toEqual(["log", "--oneline", "-2"]);
     });
+
+    it("forwards git's exit code, so a failed command is distinguishable", async () => {
+      const { box, fetchMock } = await createTestBox();
+      // 128 is what git returns when the folder is not a repository.
+      fetchMock.mockResolvedValueOnce(mockResponse({ output: "", exit_code: 128 }));
+
+      const result = await box.git.exec({ args: ["rev-parse", "--is-inside-work-tree"] });
+      expect(result.exit_code).toBe(128);
+    });
   });
 
   describe("git.checkout", () => {

packages/sdk/src/client.ts

@@ -2117,25 +2117,30 @@ export class Box<TProvider = unknown> {
           continue;
         }
 
-        // Yield any raw output before the exit event
+        // Yield any raw output before the exit event, keeping the marker at
+        // the head of the buffer in case the event itself is still arriving.
         if (exitIndex > 0) {
           yield { type: "output", data: buffer.slice(0, exitIndex) };
+          buffer = buffer.slice(exitIndex);
         }
 
-        // Parse the exit event
-        const afterEvent = buffer.slice(exitIndex + "event: exit\n".length);
-        const dataMatch = afterEvent.match(/^data:\s*(.+)/m);
-        if (dataMatch) {
-          try {
-            const parsed = JSON.parse(dataMatch[1]!);
-            yield {
-              type: "exit",
-              exitCode: parsed.exit_code ?? 0,
-              cpuNs: parsed.cpu_ns ?? 0,
-            };
-          } catch {
-            yield { type: "exit", exitCode: 0, cpuNs: 0 };
-          }
+        // The marker and its payload can land in separate network reads. A
+        // complete line is required here so that a half-arrived one is not
+        // parsed; returning at that point would end the stream with no exit
+        // chunk at all, which is what a caller uses to learn the exit status.
+        const afterEvent = buffer.slice("event: exit\n".length);
+        const dataMatch = afterEvent.match(/^data:\s*(.+)\r?\n/m);
+        if (!dataMatch) continue;
+
+        try {
+          const parsed = JSON.parse(dataMatch[1]!);
+          yield {
+            type: "exit",
+            exitCode: parsed.exit_code ?? 0,
+            cpuNs: parsed.cpu_ns ?? 0,
+          };
+        } catch {
+          yield { type: "exit", exitCode: 0, cpuNs: 0 };
         }
         return;
       }
@@ -2967,7 +2972,9 @@ export class Box<TProvider = unknown> {
   // ==================== Git (private, exposed via this.git) ====================
 
   private async _gitClone(options: GitCloneOptions): Promise<void> {
-    const folder = this._getFolder();
+    // For clone the folder is the destination, so an explicit one wins over the
+    // current directory; the directory does not exist yet by definition.
+    const folder = options.folder ?? this._getFolder();
     await this._request("POST", `/v2/box/${this.id}/git/clone`, {
       body: {
         repo: options.repo,
@@ -3013,7 +3020,7 @@ export class Box<TProvider = unknown> {
       throw new BoxError("At least one of userName or userEmail is required");
     }
 
-    return this._request<GitConfig>("PUT", `/v2/box/${this.id}/git-config`, {
+    return this._request<GitConfig>("PUT", `/v2/box/${this.id}/config/git`, {
       body: {
         git_user_name: options.userName,
         git_user_email: options.userEmail,

packages/sdk/src/types.ts

@@ -997,6 +997,12 @@ export interface GitCloneOptions {
   branch?: string;
   /** History depth (git clone --depth N); depth: 1 = shallow clone. Omit for a full clone. */
   depth?: number;
+  /**
+   * Directory under the workspace to clone into. Defaults to the repository
+   * name. Unlike the other git operations, this names the destination rather
+   * than an existing directory, so it does not have to exist yet.
+   */
+  folder?: string;
 }
 
 export interface GitExecOptions {
@@ -1005,6 +1011,8 @@ export interface GitExecOptions {
 
 export interface GitExecResult {
   output: string;
+  /** git's own exit status; 128 when the folder is not a repository. */
+  exit_code: number;
 }
 
 export interface GitCheckoutOptions {