fix(ui): gate summary built-in requests by feature flag (#10170)
## 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>
This commit is contained in:
parent
20482a4cb6
commit
106955bdfd
|
|
@ -10,6 +10,7 @@ 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")>();
|
||||
|
|
@ -23,6 +24,10 @@ 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", () => ({
|
||||
|
|
@ -90,6 +95,8 @@ describe("BuiltInAgentGate (PAP-12978)", () => {
|
|||
document.body.appendChild(container);
|
||||
listMock.mockReset();
|
||||
resumeMock.mockReset();
|
||||
getExperimentalMock.mockReset();
|
||||
getExperimentalMock.mockResolvedValue({ enableBuiltInAgents: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
|
@ -153,6 +160,16 @@ describe("BuiltInAgentGate (PAP-12978)", () => {
|
|||
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" },
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import { Button } from "@/components/ui/button";
|
|||
import { ConfigureBuiltInAgentModal } from "@/components/ConfigureBuiltInAgentModal";
|
||||
import { builtInAgentsApi, type BuiltInAgentState } from "@/api/builtInAgents";
|
||||
import { agentsApi } from "@/api/agents";
|
||||
import { instanceSettingsApi } from "@/api/instanceSettings";
|
||||
import { queryKeys } from "@/lib/queryKeys";
|
||||
import { agentUrl } from "@/lib/utils";
|
||||
import { relativeTime } from "@/lib/utils";
|
||||
|
|
@ -36,10 +37,15 @@ export function BuiltInAgentGate({ agentKey, companyId, featureLabel, children }
|
|||
const queryClient = useQueryClient();
|
||||
const [configureOpen, setConfigureOpen] = useState(false);
|
||||
|
||||
const { data: states, isLoading } = useQuery({
|
||||
const experimentalQuery = useQuery({
|
||||
queryKey: queryKeys.instance.experimentalSettings,
|
||||
queryFn: () => instanceSettingsApi.getExperimental(),
|
||||
});
|
||||
const builtInAgentsEnabled = experimentalQuery.data?.enableBuiltInAgents === true;
|
||||
const { data: states, isLoading: statesLoading } = useQuery({
|
||||
queryKey: queryKeys.builtInAgents.list(companyId ?? "__none__"),
|
||||
queryFn: () => builtInAgentsApi.list(companyId!),
|
||||
enabled: Boolean(companyId),
|
||||
enabled: Boolean(companyId && builtInAgentsEnabled),
|
||||
});
|
||||
|
||||
const state: BuiltInAgentState | undefined = states?.find((entry) => entry.definition.key === agentKey);
|
||||
|
|
@ -54,7 +60,7 @@ export function BuiltInAgentGate({ agentKey, companyId, featureLabel, children }
|
|||
|
||||
// Unknown key or still resolving the company — fail open to the feature.
|
||||
if (!companyId) return <>{children}</>;
|
||||
if (isLoading && !states) return <PageSkeleton variant="detail" />;
|
||||
if ((experimentalQuery.isLoading || statesLoading) && !states) return <PageSkeleton variant="detail" />;
|
||||
if (!state) return <>{children}</>;
|
||||
|
||||
const label = featureLabel ?? state.definition.displayName;
|
||||
|
|
|
|||
|
|
@ -230,7 +230,10 @@ describe("SummarySlotCard", () => {
|
|||
beforeEach(() => {
|
||||
container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableSummaries: true });
|
||||
mockInstanceSettingsApi.getExperimental.mockResolvedValue({
|
||||
enableSummaries: true,
|
||||
enableBuiltInAgents: true,
|
||||
});
|
||||
mockBuiltInAgentsApi.list.mockResolvedValue([readySummarizer()]);
|
||||
mockSummarySlotsApi.get.mockResolvedValue({ slot: null, document: null, generatingIssue: null } satisfies GetSummarySlotResponse);
|
||||
mockSummarySlotsApi.revisions.mockResolvedValue({ slot: null, revisions: [] } satisfies ListSummarySlotRevisionsResponse);
|
||||
|
|
@ -249,7 +252,10 @@ describe("SummarySlotCard", () => {
|
|||
});
|
||||
|
||||
it("renders nothing and does not fetch slots when the summaries flag is off", async () => {
|
||||
mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableSummaries: false });
|
||||
mockInstanceSettingsApi.getExperimental.mockResolvedValue({
|
||||
enableSummaries: false,
|
||||
enableBuiltInAgents: true,
|
||||
});
|
||||
|
||||
root = renderCard(container);
|
||||
await flushQueries();
|
||||
|
|
@ -259,6 +265,18 @@ describe("SummarySlotCard", () => {
|
|||
expect(mockBuiltInAgentsApi.list).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not query built-in agents when their feature flag is off", async () => {
|
||||
mockInstanceSettingsApi.getExperimental.mockResolvedValue({
|
||||
enableSummaries: true,
|
||||
enableBuiltInAgents: false,
|
||||
});
|
||||
|
||||
root = renderCard(container);
|
||||
await flushQueries();
|
||||
|
||||
expect(mockBuiltInAgentsApi.list).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows setup CTA when the Summarizer built-in agent needs setup", async () => {
|
||||
mockBuiltInAgentsApi.list.mockResolvedValue([needsSetupSummarizer()]);
|
||||
|
||||
|
|
|
|||
|
|
@ -154,11 +154,12 @@ export function SummarySlotCard({
|
|||
queryFn: () => instanceSettingsApi.getExperimental(),
|
||||
});
|
||||
const summariesEnabled = experimentalQuery.data?.enableSummaries === true;
|
||||
const builtInAgentsEnabled = experimentalQuery.data?.enableBuiltInAgents === true;
|
||||
|
||||
const builtInAgentsQuery = useQuery({
|
||||
queryKey: queryKeys.builtInAgents.list(companyId ?? "__none__"),
|
||||
queryFn: () => builtInAgentsApi.list(companyId!),
|
||||
enabled: Boolean(companyId && summariesEnabled),
|
||||
enabled: Boolean(companyId && summariesEnabled && builtInAgentsEnabled),
|
||||
retry: false,
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue