mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 05:16:50 +01:00
fix(ui): gate built-in agent sidebar query (#10116)
## Thinking Path > - Paperclip is the open-source control plane people use to manage AI agents and their work. > - The sidebar renders company agents and, when enabled, their built-in lifecycle state. > - The built-in-agents API is intentionally unavailable when the experimental feature is disabled. > - `SidebarAgents` nevertheless queried that endpoint whenever a company was selected, producing routine 404s in normal feature-off installations. > - The query must be gated by the shared instance setting, including its unresolved state, without exposing stale cached lifecycle badges. > - This pull request adds that gate and focused disabled, unresolved, and enabled regressions. > - The benefit is a quiet sidebar network path with unchanged behavior for installations that enable built-in agents. ## Linked Issues or Issue Description ### Pre-submission checklist - [x] I searched existing open and closed issues/PRs and found no exact duplicate. Related but not duplicate: #4149. - [x] I reproduced this on current `master`. - [x] I confirmed the error originates in Paperclip's sidebar query gating, not an adapter, provider, or local configuration. ### What happened? With `enableBuiltInAgents: false`, mounting `SidebarAgents` for a selected company still called `GET /api/companies/:companyId/built-in-agents`. The server correctly returned 404 because the experimental feature was disabled. ### Expected behavior The sidebar must not call the built-in-agent endpoint until the shared experimental setting resolves to exactly `true`. Cached built-in lifecycle state must also remain hidden while disabled. ### Steps to reproduce 1. Set `enableBuiltInAgents` to `false`. 2. Open any company so the sidebar agent list mounts. 3. Observe a request to `/api/companies/:companyId/built-in-agents` and a 404 response. ### Environment - Paperclip commit: `f2f168f6a10a24c924516808f414baba52b1c080` - Deployment mode: self-hosted server - Installation method: built from source - Adapter: not adapter-specific (core UI bug) - Database mode: not database-related - Access context: board (human operator) - Node.js: `v22.22.3` - Operating system: Linux - Relevant config: `{"enableBuiltInAgents": false}` - Relevant output: redacted HTTP 404 from `/api/companies/:companyId/built-in-agents` - Privacy: all instance-local identifiers, paths, and output were omitted or redacted. ## What Changed - Read instance experimental settings in `SidebarAgents` through the shared React Query key. - Enable the built-in-agent list query only when `enableBuiltInAgents` is explicitly `true`. - Ignore cached built-in lifecycle data while the feature is disabled. - Add regressions for disabled, unresolved, and enabled settings states. ## Verification - RED before implementation: the disabled-feature regression failed because `builtInAgentsApi.list` was called once. - `pnpm --filter @paperclipai/ui exec vitest run src/components/SidebarAgents.test.tsx` — 24 tests passed. - `pnpm --filter @paperclipai/ui exec tsc -p tsconfig.json --pretty false` — passed. - `git diff --check` — passed. - The existing test file emits pre-existing React `act(...)` warnings while passing. ## Risks - Low risk: request gating only; no API, schema, migration, or visible UI contract changes. - A stale experimental-settings cache could delay enabling the query until the normal settings invalidation/refetch path runs; this is the same shared query key already used elsewhere. - Cached built-in statuses are deliberately hidden whenever the setting is not literal `true`. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex `gpt-5.6-sol` was used with reasoning, repository/file tools, shell command execution, and delegated read-only code review. The runtime did not expose a context-window size. ## 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 linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (no documentation change needed; this enforces the existing feature flag) - [x] I have considered and documented any 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:
1 parent
2f1c0e011e
commit
b42efe2063
2 files changed
+56
-2
No files matched your search
@@ -16,6 +16,14 @@ const mockAgentsApi = vi.hoisted(() => ({
|
||||
resume: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockBuiltInAgentsApi = vi.hoisted(() => ({
|
||||
list: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockInstanceSettingsApi = vi.hoisted(() => ({
|
||||
getExperimental: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockAuthApi = vi.hoisted(() => ({
|
||||
getSession: vi.fn(),
|
||||
}));
|
||||
@@ -93,6 +101,14 @@ vi.mock("../api/agents", () => ({
|
||||
agentsApi: mockAgentsApi,
|
||||
}));
|
||||
|
||||
vi.mock("../api/builtInAgents", () => ({
|
||||
builtInAgentsApi: mockBuiltInAgentsApi,
|
||||
}));
|
||||
|
||||
vi.mock("../api/instanceSettings", () => ({
|
||||
instanceSettingsApi: mockInstanceSettingsApi,
|
||||
}));
|
||||
|
||||
vi.mock("../api/auth", () => ({
|
||||
authApi: mockAuthApi,
|
||||
}));
|
||||
@@ -222,6 +238,8 @@ describe("SidebarAgents", () => {
|
||||
mockAgentsApi.list.mockResolvedValue([makeAgent({})]);
|
||||
mockAgentsApi.pause.mockResolvedValue(makeAgent({ status: "paused" }));
|
||||
mockAgentsApi.resume.mockResolvedValue(makeAgent({}));
|
||||
mockBuiltInAgentsApi.list.mockResolvedValue([]);
|
||||
mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableBuiltInAgents: false });
|
||||
mockAuthApi.getSession.mockResolvedValue({
|
||||
session: { id: "session-1", userId: "user-1" },
|
||||
user: { id: "user-1" },
|
||||
@@ -340,6 +358,34 @@ describe("SidebarAgents", () => {
|
||||
await flushReact();
|
||||
}
|
||||
|
||||
it("does not query built-in agents when the experimental feature is disabled", async () => {
|
||||
queryClient.setQueryData(queryKeys.builtInAgents.list("company-1"), [
|
||||
{ agentId: "agent-1", status: "needs_setup" },
|
||||
]);
|
||||
|
||||
await renderSidebarAgents();
|
||||
|
||||
expect(mockBuiltInAgentsApi.list).not.toHaveBeenCalled();
|
||||
expect(container.textContent).not.toContain("Needs setup");
|
||||
});
|
||||
|
||||
it("does not query built-in agents while the experimental setting is unresolved", async () => {
|
||||
mockInstanceSettingsApi.getExperimental.mockReturnValue(new Promise(() => {}));
|
||||
|
||||
await renderSidebarAgents();
|
||||
|
||||
expect(mockBuiltInAgentsApi.list).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("queries built-in agents when the experimental feature is enabled", async () => {
|
||||
mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableBuiltInAgents: true });
|
||||
|
||||
await renderSidebarAgents();
|
||||
await flushReact();
|
||||
|
||||
expect(mockBuiltInAgentsApi.list).toHaveBeenCalledWith("company-1");
|
||||
});
|
||||
|
||||
it("renders icon-only agent rows with tooltips and no row actions in the rail", async () => {
|
||||
mockAgentsApi.list.mockResolvedValue([makeAgent({ id: "agent-a", name: "Alpha", urlKey: "alpha" })]);
|
||||
|
||||
|
||||
@@ -22,6 +22,7 @@ import { builtInAgentsApi, type BuiltInAgentStatus } from "../api/builtInAgents"
|
||||
import { BuiltInLifecycleChip } from "./BuiltInAgentBadges";
|
||||
import { authApi } from "../api/auth";
|
||||
import { heartbeatsApi } from "../api/heartbeats";
|
||||
import { instanceSettingsApi } from "../api/instanceSettings";
|
||||
import { SIDEBAR_SCROLL_RESET_STATE } from "../lib/navigation-scroll";
|
||||
import { queryKeys } from "../lib/queryKeys";
|
||||
import { cn, agentRouteRef, agentUrl, SIDEBAR_RAIL_HIDDEN_LABEL } from "../lib/utils";
|
||||
@@ -310,18 +311,25 @@ export function SidebarAgents({ streamlined = false }: { streamlined?: boolean }
|
||||
queryFn: () => agentsApi.list(selectedCompanyId!),
|
||||
enabled: !!selectedCompanyId,
|
||||
});
|
||||
const { data: experimentalSettings } = useQuery({
|
||||
queryKey: queryKeys.instance.experimentalSettings,
|
||||
queryFn: () => instanceSettingsApi.getExperimental(),
|
||||
enabled: !!selectedCompanyId,
|
||||
});
|
||||
const builtInAgentsEnabled = experimentalSettings?.enableBuiltInAgents === true;
|
||||
const { data: builtInAgents } = useQuery({
|
||||
queryKey: queryKeys.builtInAgents.list(selectedCompanyId!),
|
||||
queryFn: () => builtInAgentsApi.list(selectedCompanyId!),
|
||||
enabled: !!selectedCompanyId,
|
||||
enabled: !!selectedCompanyId && builtInAgentsEnabled,
|
||||
});
|
||||
const builtInStatusByAgentId = useMemo(() => {
|
||||
const map = new Map<string, BuiltInAgentStatus>();
|
||||
if (!builtInAgentsEnabled) return map;
|
||||
for (const entry of builtInAgents ?? []) {
|
||||
if (entry.agentId) map.set(entry.agentId, entry.status);
|
||||
}
|
||||
return map;
|
||||
}, [builtInAgents]);
|
||||
}, [builtInAgents, builtInAgentsEnabled]);
|
||||
const { data: session } = useQuery({
|
||||
queryKey: queryKeys.auth.session,
|
||||
queryFn: () => authApi.getSession(),
|
||||
|
||||
Reference in new issue
Block a user