From c0d199f97b5a81584ff4ec7091c8ee4b990112ba Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Sat, 15 Aug 2026 20:23:21 -0600 Subject: [PATCH] Cover the pin-shortcut run path and the job self-improvement footer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three zero-coverage surfaces, all reached through the queued next_focus: - useRunPinShortcut had no unit coverage at all. It is a write into a live agent session — the server resolves the pin's prompt and types it into the terminal — so the request it builds and the toast each outcome produces are worth pinning. The E2E suite proves the button reaches the endpoint once; the URL shape, the POST, and the label/error branches are only checkable here. - resolvePinShortcutIcon was covered for its name allowlist (by the existing server-side lockstep test) but not for resolution. The lookup is an own-property check specifically so inherited keys fall through to the fallback; without it "constructor" resolves to something that is not a component and throws during render. - buildSelfImprovementGuidance's job branch had no assertions anywhere. jobs/service.ts builds its prompt in a module-private function, so nothing else in the repo checks that a self-improving job is told to call update_job with its own name and directory. The template branch already reaches three suites through its caller and is not duplicated here. Mutation battery: 21 mutants, 19 killed. Two recorded survivors, neither a test gap — the `name &&` guard in resolvePinShortcutIcon is unobservable (Object.hasOwn already rejects undefined and ""), and splitting the update instruction across two lines changes guidance formatting rather than any contract, which is not worth pinning the copy to protect. Co-Authored-By: Claude Opus 5 (1M context) --- .../test/self-improvement-prompt.test.ts | 41 ++++++ apps/web/src/hooks/use-pin-shortcuts.test.tsx | 122 ++++++++++++++++++ apps/web/src/lib/pin-shortcut-icons.test.ts | 44 +++++++ 3 files changed, 207 insertions(+) create mode 100644 apps/server/test/self-improvement-prompt.test.ts create mode 100644 apps/web/src/hooks/use-pin-shortcuts.test.tsx create mode 100644 apps/web/src/lib/pin-shortcut-icons.test.ts diff --git a/apps/server/test/self-improvement-prompt.test.ts b/apps/server/test/self-improvement-prompt.test.ts new file mode 100644 index 00000000..ac985200 --- /dev/null +++ b/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); + }); +}); diff --git a/apps/web/src/hooks/use-pin-shortcuts.test.tsx b/apps/web/src/hooks/use-pin-shortcuts.test.tsx new file mode 100644 index 00000000..c89f75c6 --- /dev/null +++ b/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 ( + {children} + ); +} + +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"); + }); +}); diff --git a/apps/web/src/lib/pin-shortcut-icons.test.ts b/apps/web/src/lib/pin-shortcut-icons.test.ts new file mode 100644 index 00000000..25feeee6 --- /dev/null +++ b/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 (``) 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); + } + ); +});