mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-02 16:58:52 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The Advanced tab lets operators edit agent settings as YAML. > - Its first format embedded company catalogs, profile definitions, history, and instruction text. > - This made one agent document grow with unrelated company data. > - Removing those fields from the display also needs safe save behavior. > - This pull request renders a compact document and resolves sparse edits against saved state. > - Operators can change settings without copying shared data or deleting hidden values. ## Linked Issues or Issue Description Refs #2219. Follow-up to #18036. **Current behavior** The Advanced document expands shared profiles, embeds instruction files, and lists read-only state and unused connections. It also emits empty command lists and unset fields. **Proposed behavior** Show the agent's own settings and profile/instruction references. Omitted settings stay unchanged. Explicit empty values clear supported settings. A named instruction file changes only when supplied; `null` deletes that file. **Reason and benefit** An agent's YAML should remain readable when its company adds profiles or its instruction files grow. Every setting remains available through YAML or its owning settings tab. ## What Changed - Remove the read-only `observed` export and unused connection inventory. - Render profile-key references and instruction locations. Keep file contents and legacy prompt text out of the default editor. - Omit unset nullable fields, empty maps/lists, zero budget, standard permissions, and empty commands. - Add a sparse input schema and saved-state resolution. Preserve omitted fields, binding definitions, file contents, permissions, and channel settings. - Keep explicit clears and map/list replacement. Patch named instruction files and use `null` for deletion. Preserve hidden legacy prompt text unless explicitly changed. - Check profile references, shared edits, and file restrictions before writes. Keep revision guards, native authorization, and partial-save recovery. - Update documentation, verification maps, and regression coverage. Remove operating-system clipboard dependence from browser tests. ## Verification - All 48 jobs pass in the [final CI run](https://github.com/paperclipai/paperclip/actions/runs/40904124001) on `6dbb11d7a6`. Greptile is 5/5 on this exact commit. Both findings are resolved; there are no failed or pending checks. - Paperclip Review ended neutral because its agent finished without a complete assessment. It did not provide a code finding. Superagent Security Scan passed. - The focused suite passes 58 tests across YAML parsing, planning/saving, compact output, and the editor. - A large unit fixture with 30 profiles, 50 entries per profile, and a 1,000-line instruction file renders under 70 lines and is more than 95% smaller than the prior expanded representation. - All eight browser cases pass against real APIs and a dedicated PostgreSQL database. The compact-output case stays under 60 lines with large profiles and instruction text. It checks sparse saves and profile-key binding without changing shared definitions. The resource-save browser case also checks that unpinned skill shorthand does not trigger another sync during an unrelated save. - Browser coverage also checks resource saves/clears, secret redaction, key receipts through rename, stale drafts, lost responses, instruction relocation, and revision restore. The MCP fixture is a local target; no model run or external provider login is claimed. - Workspace build and typecheck pass. Map, Storybook catalog, and token gates pass. These checks and all eight browser cases passed after rebase onto current master, including its new Secrets regression. The merge conflict consisted of adjacent test additions; both were retained. The final skill no-op correction also passes a focused browser rerun. The mixed-resource channel regression failed before its fix and now passes, including revision restore with unpinned skills. UI typecheck passes after both corrections. - The first browser run hit a five-second test wait during an active multi-resource save. A focused rerun exposed shared clipboard interference. The helper now waits for save completion and uses scoped copy events plus keyboard input. The next full run passed. Earlier logs and screenshots remain outside the repository. Run the focused browser suite with: ```sh pnpm exec playwright test --config tests/e2e/playwright.config.ts tests/e2e/agent-configuration.spec.ts ``` ## Risks - Omitted fields now preserve saved values. Removing a line does not clear that setting. Supply its supported empty value instead. - Instruction file maps are patches. Old exports that removed a file by dropping its entry must now set that file to `null`. Entry files and read-only assets remain protected. - Explicit lists and adapter/runtime/metadata maps retain replacement behavior. Hidden legacy prompt text is preserved unless supplied explicitly; an empty string clears it. - Shared definition edits still require explicit consent. Saves still use sequential native APIs and can partially succeed. Existing unversioned resource race windows remain. - No database migration, new dependency, lockfile edit, or workflow change. ## Model Used OpenAI Codex, GPT-6 family. The exact serving model ID and context window are not exposed in this session. The work used reasoning, repository editing, code execution, and Playwright browser automation. ## 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>
154 lines
9.0 KiB
TypeScript
154 lines
9.0 KiB
TypeScript
// @vitest-environment jsdom
|
|
import { act } from "react";
|
|
import { createRoot, type Root } from "react-dom/client";
|
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
|
import { EditorView } from "@codemirror/view";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { appearanceForPalette, agentConfigurationSpec, type Agent } from "@paperclipai/shared";
|
|
import { ConfigurationSaveInterrupted } from "../lib/agent-configuration/save";
|
|
import { AgentAdvancedTab } from "./AgentAdvancedTab";
|
|
|
|
const api = vi.hoisted(() => ({ load: vi.fn(), save: vi.fn() }));
|
|
vi.mock("../lib/agent-configuration/load", async importOriginal => ({
|
|
...await importOriginal<typeof import("../lib/agent-configuration/load")>(), loadAgentConfiguration: api.load,
|
|
}));
|
|
vi.mock("../lib/agent-configuration/save", async importOriginal => ({
|
|
...await importOriginal<typeof import("../lib/agent-configuration/save")>(), saveAgentConfiguration: api.save,
|
|
}));
|
|
vi.mock("../context/ToastContext", () => ({ useToastActions: () => ({ pushToast: vi.fn() }) }));
|
|
|
|
const agent = {
|
|
id: "11111111-1111-4111-8111-111111111111", companyId: "company", urlKey: "codie", name: "Codie",
|
|
role: "engineer", title: null, icon: null, appearance: appearanceForPalette("bubblegum-sky"), reportsTo: null,
|
|
capabilities: null, adapterType: "process", adapterConfig: { command: "echo" }, runtimeConfig: {},
|
|
defaultEnvironmentId: null, budgetMonthlyCents: 0, metadata: null, configurationRevision: "a".repeat(64),
|
|
status: "idle", spentMonthlyCents: 0, pauseReason: null, pausedAt: null,
|
|
permissions: { canCreateAgents: false }, lastHeartbeatAt: null, createdAt: new Date(), updatedAt: new Date(),
|
|
} satisfies Agent & { configurationRevision: string };
|
|
|
|
function snapshot(value = agent) {
|
|
return { agent: value, core: value, document: { apiVersion: "paperclip/v1", kind: "Agent", spec: agentConfigurationSpec(value) } };
|
|
}
|
|
|
|
describe("Advanced agent configuration", () => {
|
|
let container: HTMLDivElement, root: Root, client: QueryClient;
|
|
const dirty = vi.fn(), onSaved = vi.fn();
|
|
beforeEach(() => {
|
|
vi.clearAllMocks();
|
|
Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true });
|
|
vi.stubGlobal("ResizeObserver", class { observe() {} disconnect() {} unobserve() {} });
|
|
api.load.mockResolvedValue(snapshot());
|
|
container = document.createElement("div"); document.body.appendChild(container);
|
|
root = createRoot(container);
|
|
client = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false } } });
|
|
});
|
|
afterEach(async () => { await act(() => root.unmount()); client.clear(); container.remove(); vi.unstubAllGlobals(); });
|
|
async function settle(assertion: () => void) {
|
|
for (let i = 0; i < 30; i++) {
|
|
await act(async () => { await new Promise(resolve => setTimeout(resolve, 5)); });
|
|
try { assertion(); return; } catch (error) { if (i === 29) throw error; }
|
|
}
|
|
}
|
|
async function render() {
|
|
await act(() => root.render(<QueryClientProvider client={client}>
|
|
<AgentAdvancedTab agent={agent} onDirtyChange={dirty} onSaved={onSaved} />
|
|
</QueryClientProvider>));
|
|
}
|
|
function editor() { return EditorView.findFromDOM(container.querySelector(".cm-content")!)!; }
|
|
async function edit(transform: (source: string) => string) {
|
|
await act(() => { const view = editor(); view.dispatch({ changes: { from: 0, to: view.state.doc.length, insert: transform(view.state.doc.toString()) } }); });
|
|
}
|
|
function button(label: string) { return [...container.querySelectorAll("button")].find(button => button.textContent === label)!; }
|
|
|
|
it("retains an invalid or rejected draft and reloads the saved configuration explicitly", async () => {
|
|
api.save.mockRejectedValue(new Error("Agent configuration changed while you were editing."));
|
|
await render(); await settle(() => expect(editor()).toBeTruthy());
|
|
await edit(source => `${source} budgetMonthlyCents: -1\n`);
|
|
expect(button("Save changes").disabled).toBe(true);
|
|
expect(container.querySelector('[role="alert"]')?.textContent).toContain("spec.budgetMonthlyCents");
|
|
await edit(source => source.replace("budgetMonthlyCents: -1", "budgetMonthlyCents: 500"));
|
|
await act(() => button("Save changes").click());
|
|
await settle(() => expect(container.textContent).toContain("configuration changed while you were editing"));
|
|
expect(editor().state.doc.toString()).toContain("budgetMonthlyCents: 500");
|
|
expect(api.save.mock.calls[0][1].spec.budgetMonthlyCents).toBe(500);
|
|
await act(() => button("Reload saved").click());
|
|
await settle(() => expect(editor().state.doc.toString()).not.toContain("budgetMonthlyCents:"));
|
|
expect(button("Save changes").disabled).toBe(true);
|
|
});
|
|
|
|
it("keeps the saved server result and uses its revision for the next edit", async () => {
|
|
api.save.mockResolvedValue(snapshot({ ...agent, name: "Codie Two", urlKey: "codie-two", configurationRevision: "b".repeat(64) }));
|
|
await render(); await settle(() => expect(editor()).toBeTruthy());
|
|
await edit(source => source.replace("name: Codie", "name: Codie Two"));
|
|
await act(() => button("Save changes").click());
|
|
await settle(() => expect(onSaved).toHaveBeenCalledTimes(1));
|
|
expect(button("Save changes").disabled).toBe(true);
|
|
expect(editor().state.doc.toString()).toContain("name: Codie Two");
|
|
await edit(source => `${source} title: Engineer\n`);
|
|
await act(() => button("Save changes").click());
|
|
await settle(() => expect(api.save).toHaveBeenCalledTimes(2));
|
|
expect(api.save.mock.calls[1][0].core.configurationRevision).toBe("b".repeat(64));
|
|
expect(api.save.mock.calls[1][1].spec.title).toBe("Engineer");
|
|
});
|
|
|
|
it("blocks retries after partial saves and keeps one-time credentials outside YAML", async () => {
|
|
api.save.mockImplementation(async (_snapshot, _document, onReceipt) => {
|
|
onReceipt({ id: "key", label: "Automation", token: "one-time-key" });
|
|
throw new ConfigurationSaveInterrupted("second key", ["first key"], new Error("Network lost"));
|
|
});
|
|
await render(); await settle(() => expect(editor()).toBeTruthy());
|
|
await edit(source => `${source} title: Engineer\n`);
|
|
await act(() => button("Save changes").click());
|
|
await settle(() => expect(container.textContent).toContain("Reload saved before saving again"));
|
|
expect(button("Save changes").disabled).toBe(true);
|
|
expect((container.querySelector('input[aria-label="Automation token"]') as HTMLInputElement).value).toBe("one-time-key");
|
|
expect(editor().state.doc.toString()).not.toContain("one-time-key");
|
|
await edit(source => source.replace("title: Engineer", "title: Lead"));
|
|
expect(button("Save changes").disabled).toBe(true);
|
|
await act(() => button("Reload saved").click());
|
|
await settle(() => expect(editor().state.doc.toString()).not.toContain("title:"));
|
|
await edit(source => `${source} title: Lead\n`);
|
|
expect(button("Save changes").disabled).toBe(false);
|
|
});
|
|
|
|
it("previews changes and sends only one save for a double click", async () => {
|
|
let complete!: (value: ReturnType<typeof snapshot>) => void;
|
|
api.save.mockImplementation(() => new Promise(resolve => { complete = resolve; }));
|
|
await render(); await settle(() => expect(editor()).toBeTruthy());
|
|
await edit(source => `${source} title: Engineer\n`);
|
|
expect(container.querySelector("summary")?.textContent).toBe("1 change to apply");
|
|
expect(container.querySelector("details")?.textContent).toContain("Agent configuration");
|
|
const view = editor();
|
|
await act(() => { button("Save changes").click(); button("Save changes").click(); });
|
|
await settle(() => expect(api.save).toHaveBeenCalledTimes(1));
|
|
expect(button("Reload saved").disabled).toBe(true);
|
|
await act(async () => complete(snapshot()));
|
|
await settle(() => expect(button("Save changes").disabled).toBe(true));
|
|
expect(editor()).toBe(view);
|
|
});
|
|
|
|
it("validates read-only sections before enabling Save", async () => {
|
|
api.load.mockResolvedValue({ ...snapshot(), document: { ...snapshot().document, observed: { apiKeys: [] } } });
|
|
await render(); await settle(() => expect(editor()).toBeTruthy());
|
|
await edit(source => `${source}observed:\n apiKeys: [changed]\n`);
|
|
expect(button("Save changes").disabled).toBe(true);
|
|
expect(container.textContent).toContain("observed is read-only");
|
|
expect(api.save).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("keeps Save disabled for formatting-only changes", async () => {
|
|
await render(); await settle(() => expect(editor()).toBeTruthy());
|
|
await edit(source => `${source}\n# A local note\n`);
|
|
expect(button("Save changes").disabled).toBe(true);
|
|
expect(container.textContent).toContain("No configuration changes");
|
|
expect(api.save).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("shows a configuration read denial without rendering an empty editable document", async () => {
|
|
api.load.mockRejectedValue(new Error("Configuration access denied"));
|
|
await render(); await settle(() => expect(container.textContent).toContain("Configuration access denied"));
|
|
expect(container.querySelector(".cm-content")).toBeNull();
|
|
expect(api.save).not.toHaveBeenCalled();
|
|
});
|
|
});
|