Clean up agent config environment selector (#8504)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent configuration is where operators set identity, adapter behavior, and execution environment defaults. > - The environment override control should only appear when there is a meaningful choice beyond the always-available local environment, or when an existing saved override must remain visible so it can be inspected or cleared. > - The previous UI used an "Execution" header, explanatory helper copy, and verbose inherited-default wording that made the agent config feel noisier than necessary. > - This pull request narrows the selector visibility to real alternatives and forced Kubernetes mode, then updates the visible copy to match the environment-focused mental model. > - The benefit is a cleaner agent configuration surface that only asks operators to make an environment choice when that choice exists. ## Linked Issues or Issue Description No public GitHub issue exists for this change. Public GitHub searches for related issues and PRs returned no matches: - `agent config environment selector` - `execution environment agent config` Bug-style issue description: What happened? The agent config form could surface an environment override section even when the operator had no meaningful non-local execution environment choice. The section was labeled "Execution", included helper/inheritance text, and described the inherited local default as "Inherit instance default (Local)". Expected behavior: The selector should stay hidden unless there is more than one configured environment choice, the section should be labeled "Environment", helper text should be removed, and the inherited default option should read like `Default: Local`. Existing saved non-local overrides should remain visible even if the target environment is no longer runnable, so operators can inspect or clear the stale selection. Steps to reproduce: 1. Run Paperclip from `master` with environments enabled and only the implicit local default, or Local plus one runnable non-local environment. 2. Open an agent configuration form. 3. Inspect the environment override section visibility and copy. Paperclip version or commit: `0b945f449` / current `master` before this branch. Deployment mode: Local dev (`pnpm dev`). Installation method: Built from source. Agent adapters involved: Not adapter-specific. Database mode: Not database-related. Access context: Board operator UI. ## What Changed - Shows the environment override selector only for forced Kubernetes mode, at least one runnable non-local environment, or an existing saved non-local override. - Keeps Local out of the runnable environment count so Local-only setups do not show a redundant selector. - Preserves stale saved non-local overrides in the selector options so operators can see and clear them. - Renames the section header from "Execution" to "Environment". - Removes the helper/inheritance text above the selector. - Changes the inherited option copy from `Inherit instance default (...)` to `Default: ...`. - Adds focused render tests for Local-only hiding, Local plus one runnable non-local environment showing the concise selector copy, and stale non-runnable override recovery. ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/components/AgentConfigForm.render.test.tsx` - `pnpm --filter @paperclipai/ui typecheck` - `git diff --check origin/master...HEAD` - Checked `ROADMAP.md` for overlapping planned core work. - Searched public GitHub issues and PRs for duplicate or related work; no matches found. ## Screenshots Before, when the selector was visible: ```text Execution Environment override Inheriting the instance default: Local. [Inherit instance default (Local)] ``` After, when Local plus a runnable non-local environment exists: ```text Environment Environment override [Default: Local] [E2B · sandbox] ``` After, when only Local is configured: ```text (no Environment section is rendered) ``` ## Risks Low risk. This is isolated to the agent config UI. The main behavioral risk is hiding the selector in an environment edge case; the current branch keeps forced Kubernetes mode, runnable non-local environments, and pre-existing saved non-local overrides visible. The added render tests cover the Local-only, single non-local alternative, and stale override cases. ## Model Used OpenAI Codex local adapter, GPT-5-based Codex coding session. The exact hosted backend model ID is not exposed in the runtime. Tool use and local shell execution were enabled. ## 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 - [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: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Devin Foley
GitHub
parent
0b945f449b
commit
03362b347d
|
|
@ -0,0 +1,272 @@
|
|||
// @vitest-environment jsdom
|
||||
|
||||
import { createRoot, type Root } from "react-dom/client";
|
||||
import { act } from "react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { Agent, Environment } from "@paperclipai/shared";
|
||||
import { TooltipProvider } from "@/components/ui/tooltip";
|
||||
import { AgentConfigForm } from "./AgentConfigForm";
|
||||
|
||||
const mockAgentsApi = vi.hoisted(() => ({
|
||||
adapterModels: vi.fn(),
|
||||
detectModel: vi.fn(),
|
||||
list: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockEnvironmentsApi = vi.hoisted(() => ({
|
||||
list: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockInstanceSettingsApi = vi.hoisted(() => ({
|
||||
get: vi.fn(),
|
||||
getExperimental: vi.fn(),
|
||||
getGeneral: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockSecretsApi = vi.hoisted(() => ({
|
||||
list: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../api/agents", () => ({
|
||||
agentsApi: mockAgentsApi,
|
||||
}));
|
||||
|
||||
vi.mock("../api/environments", () => ({
|
||||
environmentsApi: mockEnvironmentsApi,
|
||||
}));
|
||||
|
||||
vi.mock("../api/instanceSettings", () => ({
|
||||
instanceSettingsApi: mockInstanceSettingsApi,
|
||||
}));
|
||||
|
||||
vi.mock("../api/secrets", () => ({
|
||||
secretsApi: mockSecretsApi,
|
||||
}));
|
||||
|
||||
vi.mock("../context/CompanyContext", () => ({
|
||||
useCompany: () => ({
|
||||
companies: [{ id: "company-1", name: "Paperclip" }],
|
||||
selectedCompanyId: "company-1",
|
||||
selectedCompany: { id: "company-1", name: "Paperclip" },
|
||||
selectionSource: "bootstrap",
|
||||
loading: false,
|
||||
error: null,
|
||||
setSelectedCompanyId: vi.fn(),
|
||||
reloadCompanies: vi.fn(),
|
||||
createCompany: vi.fn(),
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../adapters", () => ({
|
||||
getUIAdapter: () => ({
|
||||
type: "codex_local",
|
||||
label: "Codex",
|
||||
ConfigFields: () => null,
|
||||
buildAdapterConfig: () => ({}),
|
||||
parseStdoutLine: () => [],
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../adapters/use-adapter-capabilities", () => ({
|
||||
useAdapterCapabilities: () => () => ({
|
||||
supportsInstructionsBundle: true,
|
||||
supportsSkills: true,
|
||||
supportsLocalAgentJwt: true,
|
||||
requiresMaterializedRuntimeSkills: false,
|
||||
supportsModelProfiles: true,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../adapters/use-disabled-adapters", () => ({
|
||||
useDisabledAdaptersSync: () => [],
|
||||
}));
|
||||
|
||||
vi.mock("./MarkdownEditor", () => ({
|
||||
MarkdownEditor: ({
|
||||
value,
|
||||
onChange,
|
||||
placeholder,
|
||||
}: {
|
||||
value: string;
|
||||
onChange: (value: string) => void;
|
||||
placeholder?: string;
|
||||
}) => (
|
||||
<textarea
|
||||
aria-label={placeholder ?? "Markdown"}
|
||||
value={value}
|
||||
onChange={(event) => onChange(event.currentTarget.value)}
|
||||
/>
|
||||
),
|
||||
}));
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
async function flushReact() {
|
||||
await act(async () => {
|
||||
for (let i = 0; i < 4; i += 1) {
|
||||
await Promise.resolve();
|
||||
await new Promise((resolve) => window.setTimeout(resolve, 0));
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
function makeAgent(overrides: Partial<Agent> = {}): Agent {
|
||||
return {
|
||||
id: "agent-1",
|
||||
companyId: "company-1",
|
||||
name: "Cody",
|
||||
role: "Engineer",
|
||||
title: null,
|
||||
icon: null,
|
||||
status: "idle",
|
||||
reportsTo: null,
|
||||
capabilities: null,
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
defaultEnvironmentId: null,
|
||||
contextMode: "thin",
|
||||
budgetMonthlyCents: 0,
|
||||
spentMonthlyCents: 0,
|
||||
permissions: {},
|
||||
lastHeartbeatAt: null,
|
||||
metadata: null,
|
||||
createdAt: new Date(0),
|
||||
updatedAt: new Date(0),
|
||||
...overrides,
|
||||
} as Agent;
|
||||
}
|
||||
|
||||
function makeEnvironment(overrides: Partial<Environment>): Environment {
|
||||
return {
|
||||
id: "env-1",
|
||||
name: "Local",
|
||||
description: null,
|
||||
driver: "local",
|
||||
status: "active",
|
||||
config: {},
|
||||
envVars: {},
|
||||
metadata: null,
|
||||
createdAt: new Date(0),
|
||||
updatedAt: new Date(0),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
async function renderForm(environments: Environment[], agentOverrides: Partial<Agent> = {}) {
|
||||
mockEnvironmentsApi.list.mockResolvedValue(environments);
|
||||
|
||||
const container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
const queryClient = new QueryClient({
|
||||
defaultOptions: {
|
||||
queries: { retry: false },
|
||||
mutations: { retry: false },
|
||||
},
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<TooltipProvider>
|
||||
<AgentConfigForm
|
||||
mode="edit"
|
||||
agent={makeAgent(agentOverrides)}
|
||||
onSave={vi.fn()}
|
||||
hidePromptTemplate
|
||||
showAdapterTypeField={false}
|
||||
showAdapterTestEnvironmentButton={false}
|
||||
/>
|
||||
</TooltipProvider>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
await flushReact();
|
||||
return { container, root };
|
||||
}
|
||||
|
||||
describe("AgentConfigForm environment selector", () => {
|
||||
let roots: Root[] = [];
|
||||
|
||||
beforeEach(() => {
|
||||
mockAgentsApi.adapterModels.mockResolvedValue([]);
|
||||
mockAgentsApi.detectModel.mockResolvedValue(null);
|
||||
mockAgentsApi.list.mockResolvedValue([]);
|
||||
mockInstanceSettingsApi.get.mockResolvedValue({ defaultEnvironmentId: null });
|
||||
mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableEnvironments: true });
|
||||
mockInstanceSettingsApi.getGeneral.mockResolvedValue({ executionMode: "any" });
|
||||
mockSecretsApi.list.mockResolvedValue([]);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
for (const root of roots) {
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
});
|
||||
}
|
||||
roots = [];
|
||||
document.body.innerHTML = "";
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("hides the environment override when Local is the only configured environment", async () => {
|
||||
const result = await renderForm([
|
||||
makeEnvironment({ id: "local-1", name: "Local", driver: "local" }),
|
||||
]);
|
||||
roots.push(result.root);
|
||||
|
||||
expect(result.container.textContent).not.toContain("Environment override");
|
||||
expect(result.container.querySelector("select")).toBeNull();
|
||||
});
|
||||
|
||||
it("shows concise Environment copy when one runnable non-local environment exists", async () => {
|
||||
const result = await renderForm([
|
||||
makeEnvironment({ id: "local-1", name: "Local", driver: "local" }),
|
||||
makeEnvironment({
|
||||
id: "sandbox-1",
|
||||
name: "E2B",
|
||||
driver: "sandbox",
|
||||
config: { provider: "e2b" },
|
||||
}),
|
||||
]);
|
||||
roots.push(result.root);
|
||||
|
||||
const text = result.container.textContent ?? "";
|
||||
const selector = result.container.querySelector("select");
|
||||
|
||||
expect(text).toContain("Environment");
|
||||
expect(text).toContain("Environment override");
|
||||
expect(selector?.textContent).toContain("Default: Local");
|
||||
expect(selector?.textContent).toContain("E2B · sandbox");
|
||||
expect(text).not.toContain("Execution");
|
||||
expect(text).not.toContain("Leave this unset to inherit the instance default");
|
||||
expect(text).not.toContain("Inherit instance default");
|
||||
});
|
||||
|
||||
it("keeps an existing non-runnable override visible so it can be cleared", async () => {
|
||||
const result = await renderForm(
|
||||
[
|
||||
makeEnvironment({ id: "local-1", name: "Local", driver: "local" }),
|
||||
makeEnvironment({
|
||||
id: "fake-sandbox-1",
|
||||
name: "Fake Sandbox",
|
||||
driver: "sandbox",
|
||||
config: { provider: "fake" },
|
||||
}),
|
||||
],
|
||||
{ defaultEnvironmentId: "fake-sandbox-1" },
|
||||
);
|
||||
roots.push(result.root);
|
||||
|
||||
const text = result.container.textContent ?? "";
|
||||
const selector = result.container.querySelector("select");
|
||||
|
||||
expect(text).toContain("Environment override");
|
||||
expect(selector?.textContent).toContain("Default: Local");
|
||||
expect(selector?.textContent).toContain("Fake Sandbox · sandbox");
|
||||
});
|
||||
});
|
||||
|
|
@ -400,10 +400,20 @@ export function AgentConfigForm(props: AgentConfigFormProps) {
|
|||
}),
|
||||
[environments, supportedEnvironmentDrivers],
|
||||
);
|
||||
const environmentOptions = useMemo(() => {
|
||||
if (!currentDefaultEnvironment) return runnableEnvironments;
|
||||
if (runnableEnvironments.some((environment) => environment.id === currentDefaultEnvironment.id)) {
|
||||
return runnableEnvironments;
|
||||
}
|
||||
return [...runnableEnvironments, currentDefaultEnvironment];
|
||||
}, [currentDefaultEnvironment, runnableEnvironments]);
|
||||
// `runnableEnvironments` excludes the always-available Local environment, so a
|
||||
// single entry already means the user has more than one environment configured
|
||||
// (Local + that environment) and the override selector is meaningful.
|
||||
const showEnvironmentOverrideControl = environmentsEnabled && (
|
||||
forcedKubernetes ||
|
||||
currentDefaultEnvironmentId.length > 0 ||
|
||||
runnableEnvironments.length > 1
|
||||
runnableEnvironments.length >= 1
|
||||
);
|
||||
const inheritedEnvironmentLabel = instanceDefaultEnvironment
|
||||
? `${instanceDefaultEnvironment.name} (${instanceDefaultEnvironment.driver})`
|
||||
|
|
@ -883,8 +893,8 @@ export function AgentConfigForm(props: AgentConfigFormProps) {
|
|||
// Render the environment read-only instead of the selectable picker.
|
||||
<div className={cn(!cards && (isCreate ? "border-t border-border" : "border-b border-border"))}>
|
||||
{cards
|
||||
? <h3 className="text-sm font-medium mb-3">Execution</h3>
|
||||
: <div className="px-4 py-2 text-xs font-medium text-muted-foreground">Execution</div>
|
||||
? <h3 className="text-sm font-medium mb-3">Environment</h3>
|
||||
: <div className="px-4 py-2 text-xs font-medium text-muted-foreground">Environment</div>
|
||||
}
|
||||
<div className={cn(cards ? "border border-border rounded-lg p-4 space-y-3" : "px-4 pb-3 space-y-3")}>
|
||||
<Field
|
||||
|
|
@ -908,20 +918,12 @@ export function AgentConfigForm(props: AgentConfigFormProps) {
|
|||
) : showEnvironmentOverrideControl ? (
|
||||
<div className={cn(!cards && (isCreate ? "border-t border-border" : "border-b border-border"))}>
|
||||
{cards
|
||||
? <h3 className="text-sm font-medium mb-3">Execution</h3>
|
||||
: <div className="px-4 py-2 text-xs font-medium text-muted-foreground">Execution</div>
|
||||
? <h3 className="text-sm font-medium mb-3">Environment</h3>
|
||||
: <div className="px-4 py-2 text-xs font-medium text-muted-foreground">Environment</div>
|
||||
}
|
||||
<div className={cn(cards ? "border border-border rounded-lg p-4 space-y-3" : "px-4 pb-3 space-y-3")}>
|
||||
<Field
|
||||
label="Environment override"
|
||||
hint="Leave this unset to inherit the instance default. Agent-specific overrides only appear when there is a real alternative."
|
||||
>
|
||||
<Field label="Environment override">
|
||||
<div className="space-y-2">
|
||||
<div className="text-xs text-muted-foreground">
|
||||
{currentDefaultEnvironment
|
||||
? `Overriding the instance default with ${currentDefaultEnvironment.name}.`
|
||||
: `Inheriting the instance default: ${inheritedEnvironmentLabel}.`}
|
||||
</div>
|
||||
<select
|
||||
className={inputClass}
|
||||
value={currentDefaultEnvironmentId}
|
||||
|
|
@ -934,8 +936,8 @@ export function AgentConfigForm(props: AgentConfigFormProps) {
|
|||
mark("identity", "defaultEnvironmentId", nextValue || null);
|
||||
}}
|
||||
>
|
||||
<option value="">Inherit instance default ({inheritedEnvironmentLabel})</option>
|
||||
{runnableEnvironments.map((environment) => (
|
||||
<option value="">Default: {inheritedEnvironmentLabel}</option>
|
||||
{environmentOptions.map((environment) => (
|
||||
<option key={environment.id} value={environment.id}>
|
||||
{environment.name} · {environment.driver}
|
||||
</option>
|
||||
|
|
|
|||
Loading…
Reference in New Issue