mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-15 12:28:10 +02:00
## Thinking Path > - Paperclip's web UI conditionally exposes experimental summary and built-in-agent capabilities. > - Summary cards depend on the built-in Summarizer agent, but the two capabilities have independent feature flags. > - `SummarySlotCard` and the reusable `BuiltInAgentGate` enabled built-in-agent lookups without requiring `enableBuiltInAgents`. > - When built-in agents were disabled, those surfaces called a server route that was intentionally unavailable and generated avoidable 404s. > - The client query should obey both server-side feature gates. > - This pull request adds the missing gate and a cross-flag regression. > - The benefit is consistent feature-flag behavior and no request loop against a disabled endpoint. ## Linked Issues or Issue Description No exact duplicate found. I searched open PRs for `SummarySlotCard`, `BuiltInAgentGate`, `enableBuiltInAgents summaries`, and `built-in agents 404`. Related PR #10116 gates `SidebarAgents`; this PR deliberately excludes that file and covers the remaining summary/gate callers. **What happened?** `SummarySlotCard` called `builtInAgentsApi.list` whenever summaries were enabled, and `BuiltInAgentGate` called it whenever a company was selected. The server rejects that route when `enableBuiltInAgents` is false, so the disabled configuration produced repeated 404 requests. **Expected behavior** Built-in-agent queries should run only when built-in agents are enabled; the summary-specific query also requires summaries to be enabled. **Steps to reproduce** 1. Enable summaries. 2. Disable built-in agents. 3. Render a page containing `SummarySlotCard` or `BuiltInAgentGate`. 4. Observe a request to the disabled built-in-agents route. **Environment** - Paperclip web UI - Cross-flag configuration: summaries enabled, built-in agents disabled - [x] I searched open PRs for the affected component, feature flags, and 404 behavior; no exact duplicate was found. ## What Changed - Require both `enableSummaries` and `enableBuiltInAgents` in `SummarySlotCard`. - Make `BuiltInAgentGate` resolve experimental settings before enabling its built-in-agent query and fail open when the feature is disabled. - Add cross-flag regressions for both callers. - Leave `SidebarAgents` to related PR #10116 rather than duplicating it. ## Verification - `pnpm exec vitest run ui/src/components/SummarySlotCard.test.tsx ui/src/components/BuiltInAgentGate.test.tsx` — passed as part of a 41-test built-in UI group. - `pnpm --filter @paperclipai/ui typecheck` — passed. - `pnpm --filter @paperclipai/ui build` — passed in combined deployment staging. - UI-only local cutover completed with the Paperclip server PID unchanged. - The complete UI fix was staged after a summary-only cutover exposed the remaining reusable-gate caller. ## Risks - Low risk: this changes only whether one query is enabled under a feature-flag combination where the server route is unavailable. - No API, schema, migration, authentication, or persistence changes. - Rollback is a single commit revert. > This is a bug fix, not roadmap feature work. ## Model Used OpenAI Codex `gpt-5.6-sol`, with tool use, code execution, repository inspection, and read-only review-agent evidence. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and described the search above - [x] I have described the issue in-PR following the bug template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have considered documentation; no user-facing documentation change is required - [x] I have considered and documented risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: cucurigoo <cucurigoo@users.noreply.github.com>
181 lines
6.3 KiB
TypeScript
181 lines
6.3 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { flushSync } from "react-dom";
|
|
import { createRoot, type Root } from "react-dom/client";
|
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
|
|
import { BuiltInAgentGate } from "./BuiltInAgentGate";
|
|
import type { BuiltInAgentState, BuiltInAgentStatus } from "@/api/builtInAgents";
|
|
|
|
const listMock = vi.hoisted(() => vi.fn());
|
|
const resumeMock = vi.hoisted(() => vi.fn());
|
|
const getExperimentalMock = vi.hoisted(() => vi.fn());
|
|
|
|
vi.mock("@/api/builtInAgents", async (importOriginal) => {
|
|
const actual = await importOriginal<typeof import("@/api/builtInAgents")>();
|
|
return {
|
|
...actual,
|
|
builtInAgentsApi: { list: listMock, provision: vi.fn(), reset: vi.fn() },
|
|
};
|
|
});
|
|
|
|
vi.mock("@/api/agents", () => ({
|
|
agentsApi: { resume: resumeMock },
|
|
}));
|
|
|
|
vi.mock("@/api/instanceSettings", () => ({
|
|
instanceSettingsApi: { getExperimental: getExperimentalMock },
|
|
}));
|
|
|
|
// The configure modal pulls in the full AgentConfigForm; stub it so the gate
|
|
// test stays focused on state selection.
|
|
vi.mock("@/components/ConfigureBuiltInAgentModal", () => ({
|
|
ConfigureBuiltInAgentModal: ({ open }: { open: boolean }) =>
|
|
open ? <div data-testid="configure-modal" /> : null,
|
|
}));
|
|
|
|
vi.mock("react-router-dom", () => ({
|
|
Link: ({ children, to }: { children: React.ReactNode; to: string }) => <a href={to}>{children}</a>,
|
|
}));
|
|
|
|
function makeState(status: BuiltInAgentStatus, overrides: Partial<BuiltInAgentState> = {}): BuiltInAgentState {
|
|
const provisioned = status !== "not_provisioned";
|
|
return {
|
|
definition: {
|
|
key: "briefs",
|
|
displayName: "Briefs Agent",
|
|
featureKeys: ["briefs"],
|
|
shortPurpose: "Prepares briefs.",
|
|
defaultInstructions: "…",
|
|
defaultRole: "general",
|
|
allowedAdapterTypes: ["codex_local"],
|
|
defaultBudgetMonthlyCents: 0,
|
|
},
|
|
status,
|
|
agentId: provisioned ? "agent-1" : null,
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
agent: provisioned
|
|
? ({ id: "agent-1", pausedAt: status === "paused" ? new Date().toISOString() : null } as any)
|
|
: null,
|
|
pauseReason: null,
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
async function flushReact() {
|
|
for (let index = 0; index < 5; index += 1) {
|
|
await Promise.resolve();
|
|
await new Promise((resolve) => window.setTimeout(resolve, 0));
|
|
}
|
|
flushSync(() => {});
|
|
}
|
|
|
|
describe("BuiltInAgentGate (PAP-12978)", () => {
|
|
let container: HTMLDivElement;
|
|
let root: Root | null = null;
|
|
|
|
async function renderGate() {
|
|
root = createRoot(container);
|
|
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
|
flushSync(() => {
|
|
root!.render(
|
|
<QueryClientProvider client={queryClient}>
|
|
<BuiltInAgentGate agentKey="briefs" companyId="c1" featureLabel="Briefs">
|
|
<div data-testid="feature">brief content</div>
|
|
</BuiltInAgentGate>
|
|
</QueryClientProvider>,
|
|
);
|
|
});
|
|
await flushReact();
|
|
}
|
|
|
|
beforeEach(() => {
|
|
container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
listMock.mockReset();
|
|
resumeMock.mockReset();
|
|
getExperimentalMock.mockReset();
|
|
getExperimentalMock.mockResolvedValue({ enableBuiltInAgents: true });
|
|
});
|
|
|
|
afterEach(() => {
|
|
flushSync(() => {
|
|
root?.unmount();
|
|
});
|
|
root = null;
|
|
container.remove();
|
|
});
|
|
|
|
it("renders the setup empty-state for needs_setup and hides the feature", async () => {
|
|
listMock.mockResolvedValue([makeState("needs_setup")]);
|
|
await renderGate();
|
|
expect(container.textContent).toContain("Set up the Briefs Agent");
|
|
expect(container.textContent).toContain("Configure its model to enable the feature");
|
|
expect(container.querySelector('[data-testid="feature"]')).toBeNull();
|
|
});
|
|
|
|
it("renders the setup empty-state for not_provisioned", async () => {
|
|
listMock.mockResolvedValue([makeState("not_provisioned")]);
|
|
await renderGate();
|
|
expect(container.textContent).toContain("Set up the Briefs Agent");
|
|
expect(container.querySelector('[data-testid="feature"]')).toBeNull();
|
|
});
|
|
|
|
it("renders a pending-approval state", async () => {
|
|
listMock.mockResolvedValue([makeState("pending_approval")]);
|
|
await renderGate();
|
|
expect(container.textContent).toContain("pending approval");
|
|
expect(container.querySelector('[data-testid="feature"]')).toBeNull();
|
|
});
|
|
|
|
it("shows the paused banner and keeps children readable (stale)", async () => {
|
|
listMock.mockResolvedValue([makeState("paused")]);
|
|
await renderGate();
|
|
expect(container.textContent).toContain("Briefs is paused.");
|
|
expect(container.textContent).toContain("Resume agent");
|
|
// Paused ≠ hidden — children still render.
|
|
expect(container.querySelector('[data-testid="feature"]')).not.toBeNull();
|
|
});
|
|
|
|
it("resumes the agent from the paused banner", async () => {
|
|
listMock.mockResolvedValue([makeState("paused")]);
|
|
resumeMock.mockResolvedValue({});
|
|
await renderGate();
|
|
const resumeButton = Array.from(container.querySelectorAll("button")).find((b) =>
|
|
b.textContent?.includes("Resume agent"),
|
|
);
|
|
expect(resumeButton).toBeTruthy();
|
|
flushSync(() => {
|
|
resumeButton!.dispatchEvent(new MouseEvent("click", { bubbles: true }));
|
|
});
|
|
await flushReact();
|
|
expect(resumeMock).toHaveBeenCalledWith("agent-1", "c1");
|
|
});
|
|
|
|
it("renders the feature when ready", async () => {
|
|
listMock.mockResolvedValue([makeState("ready")]);
|
|
await renderGate();
|
|
expect(container.querySelector('[data-testid="feature"]')).not.toBeNull();
|
|
expect(container.textContent).not.toContain("Set up the Briefs Agent");
|
|
});
|
|
|
|
it("does not query built-in agents when the feature flag is off", async () => {
|
|
getExperimentalMock.mockResolvedValue({ enableBuiltInAgents: false });
|
|
listMock.mockResolvedValue([makeState("ready")]);
|
|
|
|
await renderGate();
|
|
|
|
expect(listMock).not.toHaveBeenCalled();
|
|
expect(container.querySelector('[data-testid="feature"]')).not.toBeNull();
|
|
});
|
|
|
|
it("fails open to the feature when the key is unknown", async () => {
|
|
listMock.mockResolvedValue([makeState("ready", {
|
|
definition: { ...makeState("ready").definition, key: "learning" },
|
|
})]);
|
|
await renderGate();
|
|
expect(container.querySelector('[data-testid="feature"]')).not.toBeNull();
|
|
});
|
|
});
|