From 03362b347d669c0aa3a9cf3803361e636ce4f28d Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Mon, 22 Jun 2026 12:45:58 -0700 Subject: [PATCH] Clean up agent config environment selector (#8504) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --- .../AgentConfigForm.render.test.tsx | 272 ++++++++++++++++++ ui/src/components/AgentConfigForm.tsx | 34 +-- 2 files changed, 290 insertions(+), 16 deletions(-) create mode 100644 ui/src/components/AgentConfigForm.render.test.tsx diff --git a/ui/src/components/AgentConfigForm.render.test.tsx b/ui/src/components/AgentConfigForm.render.test.tsx new file mode 100644 index 0000000000..9d94ed5073 --- /dev/null +++ b/ui/src/components/AgentConfigForm.render.test.tsx @@ -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; + }) => ( +