Cover the pin-shortcut run path and the job self-improvement footer

apps/server/test/self-improvement-prompt.test.ts

@@ -0,0 +1,41 @@
+import { describe, expect, it } from "vitest";
+
+import { buildSelfImprovementGuidance } from "../src/shared/self-improvement-prompt.js";
+
+/**
+ * This footer is the instruction an agent follows to rewrite its own saved
+ * launch prompt, so the tool name and the identifying arguments in it are
+ * load-bearing: name the wrong tool or the wrong key and a self-improving run
+ * either does nothing or edits somebody else's configuration.
+ *
+ * Only the job branch is covered here. The template branch already reaches
+ * three suites through its caller (template-launch-prompt, templates/service,
+ * mcp-handlers), while jobs/service.ts assembles its prompt in a module-private
+ * function, so nothing else in the repo asserts the update_job instruction.
+ */
+describe("buildSelfImprovementGuidance", () => {
+  it("tells a job agent to call update_job with its own name and directory", () => {
+    const guidance = buildSelfImprovementGuidance({
+      kind: "job",
+      name: "Test Enforcer",
+      directory: "/Users/someone/dev/apps/dispatch",
+    });
+
+    expect(guidance).toContain(
+      'use update_job with name "Test Enforcer", directory "/Users/someone/dev/apps/dispatch", and the complete revised prompt.'
+    );
+    expect(guidance).not.toContain("update_template");
+  });
+
+  it("opens on its own line so it cannot run into the prompt it is appended to", () => {
+    // renderTemplatePrompt concatenates this onto the end of the prompt with
+    // no separator of its own, so the leading break has to come from here.
+    const guidance = buildSelfImprovementGuidance({
+      kind: "job",
+      name: "Test Enforcer",
+      directory: "/repo",
+    });
+
+    expect(guidance.startsWith("\nSelf-improvement:\n")).toBe(true);
+  });
+});

apps/web/src/hooks/use-pin-shortcuts.test.tsx

@@ -0,0 +1,122 @@
+// @vitest-environment jsdom
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import { act, cleanup, renderHook, waitFor } from "@testing-library/react";
+import type { ReactNode } from "react";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { useRunPinShortcut } from "./use-pin-shortcuts";
+
+vi.mock("@/lib/api", () => ({ api: vi.fn() }));
+vi.mock("sonner", () => ({
+  toast: { success: vi.fn(), error: vi.fn() },
+}));
+
+const { api } = await import("@/lib/api");
+const { toast } = await import("sonner");
+const apiMock = vi.mocked(api);
+const successMock = vi.mocked(toast.success);
+const errorMock = vi.mocked(toast.error);
+
+let queryClient: QueryClient;
+
+function wrapper({ children }: { children: ReactNode }) {
+  return (
+    <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
+  );
+}
+
+beforeEach(() => {
+  queryClient = new QueryClient({
+    defaultOptions: { mutations: { retry: false } },
+  });
+  apiMock.mockResolvedValue(null);
+});
+
+afterEach(() => {
+  cleanup();
+  queryClient.clear();
+  // restoreAllMocks only restores vi.spyOn spies, not module-factory vi.fn()s,
+  // so reset these explicitly — otherwise call history leaks between tests.
+  apiMock.mockReset();
+  successMock.mockReset();
+  errorMock.mockReset();
+});
+
+/**
+ * Firing a shortcut is a write into a live agent session: the server resolves
+ * the prompt from the pin ID and types it into the terminal. The E2E suite
+ * proves the button reaches this endpoint once; what is only checkable here is
+ * the request the hook builds and which toast each outcome produces.
+ */
+describe("useRunPinShortcut", () => {
+  it("posts to the inject-pin endpoint for the given agent and pin", async () => {
+    const { result } = renderHook(() => useRunPinShortcut(), { wrapper });
+
+    act(() => {
+      result.current.mutate({ agentId: "agt_abc", pinId: "pin_xyz" });
+    });
+
+    await waitFor(() => expect(result.current.isSuccess).toBe(true));
+    expect(apiMock).toHaveBeenCalledWith(
+      "/api/v1/agents/agt_abc/terminal/inject-pin/pin_xyz",
+      { method: "POST" }
+    );
+  });
+
+  it("names the shortcut in the success toast so stacked toasts stay distinct", async () => {
+    const { result } = renderHook(() => useRunPinShortcut(), { wrapper });
+
+    act(() => {
+      result.current.mutate({
+        agentId: "agt_abc",
+        pinId: "pin_xyz",
+        label: "Rebuild",
+      });
+    });
+
+    await waitFor(() => expect(successMock).toHaveBeenCalledTimes(1));
+    expect(successMock).toHaveBeenCalledWith('Sent "Rebuild" to agent');
+  });
+
+  it("falls back to a generic success toast when the pin has no label", async () => {
+    const { result } = renderHook(() => useRunPinShortcut(), { wrapper });
+
+    act(() => {
+      result.current.mutate({ agentId: "agt_abc", pinId: "pin_xyz" });
+    });
+
+    await waitFor(() => expect(successMock).toHaveBeenCalledTimes(1));
+    expect(successMock).toHaveBeenCalledWith("Sent to agent");
+  });
+
+  it("surfaces the server's message when the request fails", async () => {
+    apiMock.mockRejectedValue(new Error("Agent has no active session"));
+    const { result } = renderHook(() => useRunPinShortcut(), { wrapper });
+
+    act(() => {
+      result.current.mutate({
+        agentId: "agt_abc",
+        pinId: "pin_xyz",
+        label: "Rebuild",
+      });
+    });
+
+    await waitFor(() => expect(result.current.isError).toBe(true));
+    expect(errorMock).toHaveBeenCalledWith("Agent has no active session");
+  });
+
+  it("falls back to generic copy when the rejection is not an Error", async () => {
+    // api() rejects with an Error on every HTTP path, but a network-layer
+    // throw or a future caller need not — and a bare string would render as
+    // "undefined" in the toast if the message were read off it blindly.
+    apiMock.mockRejectedValue("boom");
+    const { result } = renderHook(() => useRunPinShortcut(), { wrapper });
+
+    act(() => {
+      result.current.mutate({ agentId: "agt_abc", pinId: "pin_xyz" });
+    });
+
+    await waitFor(() => expect(result.current.isError).toBe(true));
+    expect(errorMock).toHaveBeenCalledWith("Failed to send shortcut");
+  });
+});

apps/web/src/lib/pin-shortcut-icons.test.ts

@@ -0,0 +1,44 @@
+import { GitPullRequest, Trash2, Zap } from "lucide-react";
+import { describe, expect, it } from "vitest";
+
+import { resolvePinShortcutIcon } from "./pin-shortcut-icons";
+
+/**
+ * The name allowlist itself is pinned by the server-side lockstep test
+ * (apps/server/test/pin-shortcut-icons-lockstep.test.ts), which compares this
+ * module's keys against the dispatch_pin schema. What is left uncovered — and
+ * what this file covers — is the resolution step: an agent supplies an
+ * arbitrary string, and every path out of it has to yield a real component,
+ * because the caller renders the result directly (`<Icon />`) with no guard.
+ */
+describe("resolvePinShortcutIcon", () => {
+  it("resolves a name from the allowlist to its component", () => {
+    expect(resolvePinShortcutIcon("trash")).toBe(Trash2);
+  });
+
+  it("resolves a hyphenated name", () => {
+    expect(resolvePinShortcutIcon("pull-request")).toBe(GitPullRequest);
+  });
+
+  it("falls back to the zap glyph when no icon was set", () => {
+    expect(resolvePinShortcutIcon(undefined)).toBe(Zap);
+  });
+
+  it("falls back for a name that is not on the allowlist", () => {
+    expect(resolvePinShortcutIcon("nonexistent")).toBe(Zap);
+  });
+
+  it("falls back for an empty name rather than treating it as a lookup", () => {
+    expect(resolvePinShortcutIcon("")).toBe(Zap);
+  });
+
+  // The lookup is an own-property check precisely so these fall through: `in`
+  // or a bare `PIN_SHORTCUT_ICONS[name]` would hand the caller Object.prototype
+  // members, which are not components and throw during render.
+  it.each(["constructor", "__proto__", "toString", "hasOwnProperty"])(
+    "falls back for the inherited key %j",
+    (name) => {
+      expect(resolvePinShortcutIcon(name)).toBe(Zap);
+    }
+  );
+});