fix(dashboard): allow deleting the last extra-upstream-header row even when invalid (#12251)

changelog.d/fixes/12251-extra-upstream-headers-delete.md

@@ -0,0 +1 @@
+- fix(dashboard): allow deleting the last extra-upstream-header row even when invalid (#12251)

changelog.d/fixes/12645-auggie-cli-not-found-shell-exit.md

@@ -1 +0,0 @@
-- fix(sse): surface the actionable "Auggie CLI not found" message when the shell reports a missing `auggie` binary via exit code instead of a spawn error (#12645)

open-sse/executors/auggie.ts

@@ -294,24 +294,6 @@ function isEnoentLike(message: string): boolean {
   return message.includes("ENOENT") || message.includes("not found");
 }
 
-// Windows cmd.exe and POSIX shells never raise a Node `spawn` 'error' event for a
-// missing binary when `shell: true` is used (see buildAuggieSpawnOptions) — they
-// report it as a normal non-zero exit with the "not found" text on stderr instead.
-// Recognize that shape too so the `close` handlers give the same actionable
-// cliNotFoundMessage() as the `error` handlers already do. See #12645.
-const CLI_NOT_FOUND_STDERR_PATTERNS = [
-  /is not recognized as an internal or external command/i,
-  /command not found/i,
-  // dash/POSIX `sh` shells report a missing executable as `<name>: not found`
-  // (no literal "command"), e.g. "sh: 1: auggie: not found".
-  /:\s*not found\s*$/im,
-  /No such file or directory/i,
-];
-
-function isCliNotFoundText(stderrTail: string): boolean {
-  return CLI_NOT_FOUND_STDERR_PATTERNS.some((pattern) => pattern.test(stderrTail));
-}
-
 export type AuggieCliVersionCheck = { ok: boolean; version?: string; error?: string };
 
 /**
@@ -598,11 +580,9 @@ export class AuggieExecutor extends BaseExecutor {
           if (finished) return;
           if (code !== 0) {
             emitError(
-              isCliNotFoundText(stderrTail)
-                ? cliNotFoundMessage(auggieBin)
-                : sanitizeErrorMessage(
-                    `Auggie CLI exited with code ${code}${stderrTail ? `: ${stderrTail}` : ""}`
-                  )
+              sanitizeErrorMessage(
+                `Auggie CLI exited with code ${code}${stderrTail ? `: ${stderrTail}` : ""}`
+              )
             );
             return;
           }
@@ -684,11 +664,9 @@ export class AuggieExecutor extends BaseExecutor {
         if (code !== 0) {
           settle(
             buildAuggieErrorResponse(
-              isCliNotFoundText(stderrTail)
-                ? cliNotFoundMessage(auggieBin)
-                : sanitizeErrorMessage(
-                    `Auggie CLI exited with code ${code}${stderrTail ? `: ${stderrTail}` : ""}`
-                  )
+              sanitizeErrorMessage(
+                `Auggie CLI exited with code ${code}${stderrTail ? `: ${stderrTail}` : ""}`
+              )
             )
           );
           return;

src/app/(dashboard)/dashboard/providers/[id]/components/ModelCompatPopover.tsx

@@ -707,7 +707,10 @@ export default function ModelCompatPopover({
                       </div>
                       <button
                         type="button"
-                        disabled={disabled || headerRows.length <= 1}
+                        disabled={
+                          disabled ||
+                          (headerRows.length <= 1 && !row.name.trim() && !row.value.trim())
+                        }
                         onClick={() => removeHeaderRow(row.id)}
                         title={t("compatUpstreamRemoveRow")}
                         className="flex h-9 w-9 shrink-0 items-center justify-center rounded-lg border border-border/80 text-text-muted hover:bg-red-500/10 hover:text-red-600 dark:hover:text-red-400 disabled:opacity-30 disabled:hover:bg-transparent disabled:hover:text-text-muted transition-colors"

src/app/(dashboard)/dashboard/providers/[id]/components/__tests__/modelCompatPopover-12251-delete-invalid-header.test.tsx

@@ -0,0 +1,113 @@
+// @vitest-environment jsdom
+// Repro for #12251
+import React, { act } from "react";
+import { createRoot, type Root } from "react-dom/client";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import ModelCompatPopover from "../ModelCompatPopover";
+
+vi.mock("next-intl", () => ({
+  useTranslations: () => (key: string) => key,
+}));
+
+let container: HTMLDivElement;
+let root: Root;
+
+async function flushEffects() {
+  await act(async () => {
+    await Promise.resolve();
+  });
+}
+
+async function openPopover() {
+  const trigger = container.querySelector("button") as HTMLButtonElement;
+  await act(async () => trigger.click());
+  await flushEffects();
+}
+
+describe("ModelCompatPopover upstream headers — invalid single row cannot be deleted (#12251)", () => {
+  beforeEach(() => {
+    (
+      globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }
+    ).IS_REACT_ACT_ENVIRONMENT = true;
+    container = document.createElement("div");
+    document.body.appendChild(container);
+    root = createRoot(container);
+  });
+
+  afterEach(() => {
+    act(() => root.unmount());
+    document.body.innerHTML = "";
+    vi.unstubAllGlobals();
+  });
+
+  it("lets the user delete the invalid header via the delete icon when it is the ONLY row present", async () => {
+    const onCompatPatch = vi.fn();
+
+    act(() => {
+      root.render(
+        <ModelCompatPopover
+          t={(key) => key}
+          providerId="openai"
+          modelId="gpt-test"
+          effectiveModelNormalize={() => false}
+          effectiveModelPreserveDeveloper={() => true}
+          getUpstreamHeadersRecord={() => ({
+            "https://evil.example.com/callback": "some-secret-value",
+          })}
+          onCompatPatch={onCompatPatch}
+        />
+      );
+    });
+
+    await openPopover();
+
+    const nameInput = document.querySelector(
+      'input[placeholder="compatUpstreamHeaderNamePlaceholder"]'
+    ) as HTMLInputElement;
+    expect(nameInput).toBeTruthy();
+    expect(nameInput.value).toBe("https://evil.example.com/callback");
+
+    const rowButtons = document.querySelectorAll('button[title="compatUpstreamRemoveRow"]');
+    expect(rowButtons.length).toBe(1);
+
+    const removeButton = rowButtons[0] as HTMLButtonElement;
+
+    // EXPECTED (fixed) behavior: a populated row should always be removable via
+    // its own delete icon, even when it is the only row.
+    expect(removeButton.disabled).toBe(false);
+
+    await act(async () => removeButton.click());
+    await flushEffects();
+
+    expect(onCompatPatch).toHaveBeenCalledWith("openai", { upstreamHeaders: {} });
+  });
+
+  it("keeps the delete button disabled when the sole row is genuinely blank", async () => {
+    const onCompatPatch = vi.fn();
+
+    act(() => {
+      root.render(
+        <ModelCompatPopover
+          t={(key) => key}
+          providerId="openai"
+          modelId="gpt-test"
+          effectiveModelNormalize={() => false}
+          effectiveModelPreserveDeveloper={() => true}
+          getUpstreamHeadersRecord={() => ({})}
+          onCompatPatch={onCompatPatch}
+        />
+      );
+    });
+
+    await openPopover();
+
+    const rowButtons = document.querySelectorAll('button[title="compatUpstreamRemoveRow"]');
+    expect(rowButtons.length).toBe(1);
+
+    const removeButton = rowButtons[0] as HTMLButtonElement;
+
+    // A blank sole row must stay non-deletable so the form always shows an
+    // editable add-affordance.
+    expect(removeButton.disabled).toBe(true);
+  });
+});

tests/unit/auggie-cli-not-found-shell-exit-12645.test.ts

@@ -1,103 +0,0 @@
-import test from "node:test";
-import assert from "node:assert/strict";
-import fs from "node:fs";
-import os from "node:os";
-import path from "node:path";
-
-import type { ExecuteInput } from "@omniroute/open-sse/executors/base";
-
-const { AuggieExecutor, __resetAuggieModels } = await import(
-  "@omniroute/open-sse/executors/auggie"
-);
-
-function makeFakeAuggieBin(dir: string, stderrLine: string): string {
-  const fakeBin = path.join(dir, "fake-auggie.sh");
-  fs.writeFileSync(fakeBin, `#!/bin/sh\necho "${stderrLine}" 1>&2\nexit 1\n`);
-  fs.chmodSync(fakeBin, 0o755);
-  return fakeBin;
-}
-
-async function withFakeAuggieBin<T>(
-  stderrLine: string,
-  fn: (dir: string) => Promise<T>
-): Promise<T> {
-  const dir = fs.mkdtempSync(path.join(os.tmpdir(), "auggie-probe-"));
-  const fakeBin = makeFakeAuggieBin(dir, stderrLine);
-  const prevBin = process.env.AUGGIE_BIN;
-  process.env.AUGGIE_BIN = fakeBin;
-  __resetAuggieModels();
-  try {
-    return await fn(dir);
-  } finally {
-    if (prevBin === undefined) delete process.env.AUGGIE_BIN;
-    else process.env.AUGGIE_BIN = prevBin;
-    __resetAuggieModels();
-    fs.rmSync(dir, { recursive: true, force: true });
-  }
-}
-
-test("Auggie CLI-not-found surfaced via shell exit code (non-streaming) gets the actionable cliNotFoundMessage", async () => {
-  await withFakeAuggieBin(
-    "'auggie' is not recognized as an internal or external command,",
-    async () => {
-      const executor = new AuggieExecutor();
-      const { response } = await executor.execute({
-        model: "",
-        body: { messages: [{ role: "user", content: "hi" }] },
-        stream: false,
-        credentials: {} as never,
-      } satisfies ExecuteInput);
-
-      const json = await response.json();
-      const message: string = json?.error?.message ?? "";
-
-      // sanitizeErrorMessage() redacts the absolute bin path (and anything the
-      // path-redaction tokenizer folds into it) — see errorPathRedaction.ts —
-      // so the assertion mirrors the existing precedent in
-      // auggie-executor.test.ts: assert the actionable prefix routed through
-      // cliNotFoundMessage(), and that the raw, confusing shell text from
-      // #12645 is gone.
-      assert.match(
-        message,
-        /Auggie CLI not found/,
-        `expected the actionable 'Auggie CLI not found' message, but got: ${message}`
-      );
-      assert.doesNotMatch(
-        message,
-        /is not recognized as an internal or external command/i,
-        `expected the raw shell text to be replaced, but got: ${message}`
-      );
-    }
-  );
-});
-
-test("Auggie CLI-not-found surfaced via shell exit code (streaming) gets the actionable cliNotFoundMessage", async () => {
-  await withFakeAuggieBin("sh: 1: auggie: not found", async () => {
-    const executor = new AuggieExecutor();
-    const { response } = await executor.execute({
-      model: "",
-      body: { messages: [{ role: "user", content: "hi" }] },
-      stream: true,
-      credentials: {} as never,
-    } satisfies ExecuteInput);
-
-    const text = await response.text();
-    const dataLine = text
-      .split("\n")
-      .find((line) => line.startsWith("data: ") && line.includes('"error"'));
-    assert.ok(dataLine, `expected an SSE error frame, got body: ${text}`);
-    const payload = JSON.parse(dataLine!.slice("data: ".length));
-    const message: string = payload?.error?.message ?? "";
-
-    assert.match(
-      message,
-      /Auggie CLI not found/,
-      `expected the actionable 'Auggie CLI not found' message, but got: ${message}`
-    );
-    assert.doesNotMatch(
-      message,
-      /exited with code/i,
-      `expected the raw shell exit-code text to be replaced, but got: ${message}`
-    );
-  });
-});