From cc6a3296e5341ec1a1e3c895d4c3e0bb5e7d22ec Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:17:17 -0500 Subject: [PATCH] fix: keep agent YAML compact and preserve omitted settings (#18168) ## 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 --- doc/agent-configuration-yaml.md | 152 +++++++-------- maps/agent-access-map/agents.md | 5 +- maps/feature-map/agents.md | 10 +- .../src/agent-configuration-resources.ts | 30 +++ packages/shared/src/agent-configuration.ts | 6 +- tests/e2e/agent-configuration.spec.ts | 124 ++++++++---- ui/src/lib/agent-configuration-yaml.test.ts | 5 +- ui/src/lib/agent-configuration-yaml.ts | 12 +- ui/src/lib/agent-configuration/channels.ts | 4 +- ui/src/lib/agent-configuration/compact.ts | 66 +++++++ ui/src/lib/agent-configuration/load.ts | 21 +- ui/src/lib/agent-configuration/resolve.ts | 61 ++++++ ui/src/lib/agent-configuration/save.test.ts | 183 +++++++++++++++++- ui/src/lib/agent-configuration/save.ts | 17 +- ui/src/lib/agent-configuration/tools.ts | 4 +- ui/src/pages/AgentAdvancedTab.test.tsx | 16 +- ui/src/pages/AgentAdvancedTab.tsx | 12 +- 17 files changed, 545 insertions(+), 183 deletions(-) create mode 100644 ui/src/lib/agent-configuration/compact.ts create mode 100644 ui/src/lib/agent-configuration/resolve.ts diff --git a/doc/agent-configuration-yaml.md b/doc/agent-configuration-yaml.md index c985b734750..080661c3bb6 100644 --- a/doc/agent-configuration-yaml.md +++ b/doc/agent-configuration-yaml.md @@ -1,6 +1,6 @@ # Agent configuration YAML -Status: v1, October 28, 2026. +Status: v1, compact editor updated October 29, 2026. The **Advanced** tab at `/:companyPrefix/agents/:agentRef/advanced` edits an agent's settings as one YAML document. It covers the configuration tabs through @@ -13,14 +13,14 @@ this is not a file on the agent's computer or a harness-specific format. | --- | --- | --- | | Agent identity / profile | `spec.name`, `role`, `title`, `icon`, `appearance`, `reportsTo`, `capabilities`, `metadata` | Existing agent update, including avatar appearance and reporting relationships. | | Harness / Runtime | `spec.adapterType`, `adapterConfig`, `runtimeConfig`, `defaultEnvironmentId`, `budgetMonthlyCents` | Existing adapter, environment, AI connection, heartbeat, run-policy and budget validation. Adapter-specific extensions are preserved. | -| Instructions | `spec.instructions` | Bundle mode, root, entry file, and editable text files. Managed files and entry revisions enforce concurrency preconditions; omitted text files are deleted. Legacy prompt templates remain in `adapterConfig`. | +| Instructions | `spec.instructions` | Bundle mode, root, entry file, and editable text files. Managed files and entry revisions enforce concurrency preconditions; omitted files are preserved; `null` deletes a named file. Legacy prompt templates stay in Instructions unless explicitly supplied. | | Skills | `spec.skills` | Desired skill keys and version pins, synchronized through the skills API. | | Secrets & variables | `spec.adapterConfig.env`, `adapterConfig["access."]`, `spec.secrets` | Environment values, secret references, API-delivery bindings, creation, rotation, metadata/status updates, and proposal decisions. | -| Tools | `spec.tools` | Direct connection installs, profile bindings and definitions, and agent-scoped policies. Inherited access is visible under `observed`. | +| Tools | `spec.tools` | Direct connection installs, profile bindings and definitions, and agent-scoped policies. Inherited access remains in the Tools tab. | | Channels | `spec.channels`, `spec.channelOperations` | Existing endpoint settings, communication instructions, execution defaults, resource selection, setup/credentials and lifecycle actions, creation, and identity revocation. | | Permissions / Trust | `spec.permissions` | Agent creation, skill creation, task assignment, trust preset and full authorization policy, including allowed projects in `authorizationPolicy.trustBoundary.projectIds`. | -| API Keys | `spec.apiKeys` | Explicit create and revoke operations; key metadata is under `observed.apiKeys`. New tokens appear once outside the YAML. | -| Revisions | `spec.revisions.restore` | Restore an existing revision, as the only operation in a save; history is under `observed.revisions`. | +| API Keys | `spec.apiKeys` | Explicit create and revoke operations; key metadata remains in the API Keys tab. New tokens appear once outside the YAML. | +| Revisions | `spec.revisions.restore` | Restore an existing revision, as the only operation in a save; history remains in the Revisions tab. | Read-only information stays read-only: organization-wide inherited access, immutable history, binary instruction assets, live status/spend, and provider @@ -37,15 +37,6 @@ kind: Agent spec: name: Codie role: engineer - title: Software engineer - icon: null - appearance: - schemaVersion: 1 - characterVersion: cap-v1 - paletteId: bubblegum-sky - reportsTo: null - capabilities: | - Implement features, review code, and maintain tests. adapterType: codex_local adapterConfig: model: gpt-5 @@ -59,61 +50,40 @@ spec: heartbeat: enabled: true intervalSec: 3600 - wakeOnDemand: true - defaultEnvironmentId: null budgetMonthlyCents: 10000 - metadata: null - permissions: - canCreateAgents: false - canCreateSkills: true - canAssignTasks: false - trustPreset: standard instructions: mode: managed - rootPath: /managed/agents/codie entryFile: AGENTS.md - files: - AGENTS.md: | - # Codie - Read the task, make focused changes, and verify the result. - references/workflow.md: | - Run the relevant tests before handing work back. skills: - - key: paperclip - versionId: null + - paperclip tools: - connections: [] - profiles: [] - policies: [] - channels: [] - secrets: - create: [] - rotate: [] - update: [] - proposals: [] - channelOperations: - create: [] - setup: [] - revokeIdentityLinks: [] - apiKeys: - create: [] - revoke: [] - revisions: - restore: null -observed: {} + profiles: + - profileKey: engineering ``` -This illustrates the format; model, paths, and secret IDs depend on the -installation. Edit the document loaded for the agent. Do not replace its -`observed` block with the example's empty block. +This illustrates the compact format; model, paths, and references depend on the +installation. The default editor contains agent-specific settings and references. +It does not embed company catalogs, inherited profile definitions, history, +credential metadata, or instruction file contents. Empty commands, unset nullable +fields, zero budget, empty maps/lists, and standard permission defaults are omitted. +Unpinned skills use their key strings. Non-default values remain explicit. -`apiVersion`, `kind`, `spec`, and every core field from `name` through `metadata` -are required, including explicit `null` for unset nullable fields. Resource -sections (`permissions` onward) are optional: omitting an entire section -preserves it. The editor emits all -sections. Unknown fields at the document, spec, or structured resource level -are errors. Open adapter/runtime/metadata maps retain their existing extension -contracts. Future incompatible changes require a new API version. +`apiVersion`, `kind`, and `spec` are required. All settings inside `spec` are +optional when editing an existing agent. Omission preserves the saved value, +including omitted instruction files, tool subsections, channel settings, and +permissions. To clear a value, supply its supported empty value explicitly: +`title: null`, `skills: []`, `tools: { profiles: [] }`, or `adapterConfig: {}`. +An explicit list replaces that list. An explicit adapter/runtime/metadata map +replaces that map. The one adapter exception is hidden legacy `promptTemplate`: +omission preserves it, and an explicit empty string clears it. + +All settings remain expressible. Add an optional section when needed. Use the +owning settings tabs to inspect instruction contents, available profiles, +inherited access, API key metadata, secret proposals, and revision history. +A supplied legacy `observed` block is read-only; remove it before editing an old +export. Unknown fields remain errors. Open adapter/runtime/metadata maps retain +their existing extension contracts. Future incompatible changes require a new +API version. Executable contracts: [core document](../packages/shared/src/agent-configuration.ts) and @@ -127,12 +97,16 @@ secret, channel, skill, and tool validators. `instructions.mode` is `managed`, `external`, or `null` for an unconfigured bundle. `external` requires an absolute `rootPath`. File names and `entryFile` -are relative paths; traversal and absolute file names are rejected. Each file -value is its text content (up to 1 MiB). Adding a mapping creates a file; -changing one edits it; removing one deletes it. The entry file cannot be -removed. Binary/read-only/virtual files are described in -`observed.instructionFiles`. A legacy virtual prompt template is edited using -`adapterConfig.promptTemplate` rather than a second writable copy. +are relative paths; traversal and absolute file names are rejected. File +contents are omitted from the default YAML. The Instructions tab owns +normal content editing. To edit a file inline, add `files: { workflow.md: "text" }` +under `instructions`; only named files are changed. Each text value supports up +to 1 MiB. Use `files: { workflow.md: null }` to delete that file. Omitted files +and an empty `files: {}` are preserved. The entry file cannot be removed. +Binary/read-only/virtual files remain visible in Instructions. A legacy virtual +prompt template can still be edited explicitly with `adapterConfig.promptTemplate`. +It is hidden by default and preserved when another adapter setting changes. +Managed root paths are assigned by Paperclip and omitted from the default YAML. Changing the mode, root, or entry file is a separate save from editing file contents. That save loads the destination files without writing the old YAML's @@ -152,15 +126,23 @@ sections cannot disagree about the same setting. ### Tools -`tools.connections` contains `{ connectionId, installed }` for each available -connection. These are direct agent installations; inherited company -installations appear in `observed.connections`. Removing a direct installation +`tools.connections` contains only selected direct agent installations. +Each row has `connectionId`; omitted `installed` means `true`. Use +`installed: false` or remove a row from an explicitly supplied list to uninstall. +Inherited company installations remain in Tools. Removing a direct installation row has the same effect as `installed: false`. Saves preserve other agents' installations. Company installations cannot be overridden here. The installation service can add an access profile and binding for an installed -connection. The canonical reload includes those generated profiles. +connection. The compact reload references those generated profiles by key. -Each profile contains: +Existing profiles normally use a reference: `{ profileKey: engineering }`. +`profileId` is also accepted. If both are provided, they must identify the same +company profile. Optional `priority` and `metadata` affect only this binding; +omission preserves an existing binding or uses 100/null for a new one. +A reference never updates the shared definition. Omit `tools.profiles` to +preserve bindings; use `profiles: []` to unbind all direct profiles. + +To create a profile or explicitly edit its definition, supply: ```yaml profileId: null # null creates a profile; an existing ID binds/edits it @@ -186,8 +168,8 @@ When other targets share it, changes to the definition require explicitly setting `updateSharedDefinition: true`. The flag resets after reload; changing only the flag does not write anything. Binding an unchanged shared definition does not require the flag. Use `profileId: null` for a separate profile. -Removing a profile row only unbinds it from this agent. Available profile -configurations are listed under `observed.availableProfiles`. +Removing a profile row from an explicitly supplied list only unbinds it from +this agent. Available profile definitions remain in Tools. Keep existing profile and policy fields explicit. Removing a field from an existing definition is rejected because the native PATCH would otherwise @@ -197,17 +179,16 @@ Policies use `{ id, configuration }`, with `id: null` creating a policy. Configuration includes `name`, `description`, `policyType`, `priority`, `enabled`, `selectors`, `conditions`, and `config`. Policies in this section must target only this agent; newly created policies are scoped to it. Removing -a row deletes that agent-scoped policy. Shared/inherited policies are read-only -under `observed.inheritedPolicies`. +a row deletes that agent-scoped policy. Shared/inherited policies remain read-only in Tools. ### Channels -Each `channels` row has immutable `id` and `provider`, plus `settings` and -`resources`. Settings follow the native endpoint update contract: +Each `channels` row has immutable `id`; `provider` can be omitted or must match +the saved provider. Omitted `settings` and `resources` preserve their saved values. Settings follow the native endpoint update contract: `executionDefaults`, `communicationInstructions`, `allowUnlinkedPeople`, `allowDirectMessages`, `requireAtMention`, `allowGroupChats`, `slackApp`, and -`slackSetupMethod`, where applicable. Keep saved settings explicit; omission -cannot silently clear a PATCH field. Resources contain `{ id, enabled, +`slackSetupMethod`, where applicable. Omitted settings fields are preserved; an explicit empty value clears a field +where its native API supports it. Resources contain `{ id, enabled, executionDefaults }`; removing a resource row disables it. Removing a channel row removes the endpoint using its existing setup API. @@ -219,7 +200,7 @@ has an `action` (`configure`, `verify`, `pause`, `resume`, `reconnect`, `remove` and optional `credentials` or Photon configuration. `revokeIdentityLinks` accepts `{ endpointId, principalId }`. Provider authorization is still enforced by the channel service. IDs, resources, linked people, and state are available -under `observed.channels`. +in the Channels tab. An endpoint cannot be removed and also edited, configured, or have its identities revoked in the same save. Duplicate setup commands and identity revocations are @@ -234,8 +215,7 @@ provider configuration and metadata). After creation, `bind` can attach its ID to environment names or API aliases. An empty bind creates without attaching. `rotate` and `update` accept `{ id, input }` using their native contracts. `proposals` accepts `{ id, decision: approve|reject, reason? }`; rejection -requires a reason. Secrets and pending proposals are listed without values -under `observed.secrets` and `observed.secretProposals`. +requires a reason. Secrets and pending proposals remain in Secrets & variables. New secrets must use distinct binding targets. A target cannot also be edited in `adapterConfig` in the same save. Binding targets are checked again before @@ -255,9 +235,8 @@ the only change in that save. It uses the same rollback operation as the Revisions tab and restores only the fields covered by that revision; it is not a rollback of all external resources. -Operation lists reset to empty after a successful save/reload. Deleting an -`observed` history row does not revoke a key or restore a revision: editing -`observed` is rejected. Lifecycle commands for the agent itself (pause, resume, +Empty operation lists are omitted after a successful save/reload. Commands are +not replayed from history. Read-only history and catalogs are excluded from YAML. Lifecycle commands for the agent itself (pause, resume, terminate) are actions outside configuration. ## YAML, secrets, and save guarantees @@ -335,7 +314,8 @@ Run the browser acceptance suite against its disposable checkout instance: pnpm exec playwright test --config tests/e2e/playwright.config.ts tests/e2e/agent-configuration.spec.ts ``` -The suite checks resource saves and clears, one-time key receipts, secret +The suite checks compact output with large company catalogs and instruction +files, sparse saves, profile-key bindings, resource saves and explicit clears, one-time key receipts, secret redaction, stale drafts, a lost creation response, instruction relocation, and revision restore through real APIs. It enables and restores the Beta skills and chat connector flags. Its local MCP fixture provides a connection target; it diff --git a/maps/agent-access-map/agents.md b/maps/agent-access-map/agents.md index b3a2804d83d..1078fe50d1c 100644 --- a/maps/agent-access-map/agents.md +++ b/maps/agent-access-map/agents.md @@ -112,7 +112,10 @@ The operator's Advanced YAML editor uses the existing configuration GET/PATCH operations plus the owning instructions, skills, tools, channels, permissions, secrets, API key, and revision APIs. It does not add aggregate write authority; each request retains its own company and actor checks. Resource reads fail closed. -Observed inherited access/history is read-only. Multi-resource writes are +The compact YAML editor omits inherited access/history, company catalogs, and +instruction contents. Profile-key references resolve only within the company. +Omitted settings are preserved; named file patches use explicit null for deletion. +The normal domain APIs still enforce every write. Multi-resource writes are sequential, with preflight comparison and explicit partial-failure recovery; Core configuration, managed files, and entry revisions enforce their existing atomic revision/hash preconditions. External supporting files and other diff --git a/maps/feature-map/agents.md b/maps/feature-map/agents.md index b745f8de2dd..d8abec04779 100644 --- a/maps/feature-map/agents.md +++ b/maps/feature-map/agents.md @@ -52,17 +52,21 @@ resource allocation waits for approval. Open `/agents/:agentId` and the configuration/instruction surfaces it exposes. -Open **Advanced** at `/agents/:agentId/advanced` for the complete agent +Open **Advanced** at `/agents/:agentId/advanced` for the compact agent [YAML configuration](../../doc/agent-configuration-yaml.md). Edit a name, budget, runtime setting, and adapter key; save, reload, and check the ordinary settings -and Revisions. Try malformed YAML, a missing/unknown field, a removed nested key, +and Revisions. Try malformed YAML, an unknown field, omitted settings, a removed adapter key, and a concurrent settings edit. Errors must retain the draft; stale saves return a conflict. Unchanged secret markers must preserve their stored values. Navigating away with a draft must show the existing unsaved-change guard. **Reload saved** discards the draft. Also edit instruction text, skill pins, direct tool installs, channel settings/resources, permissions/trust/project boundaries, secret operations, and API key operations. Verify each result in its owning tab. Revision restore is -an isolated save. Inherited access/history under `observed` is read-only. An +an isolated save. Default YAML excludes catalogs, inherited definitions, history, and instruction +contents. Verify profile-key references, named instruction-file patches (`null` +deletes one file), omitted-setting preservation, and explicit clears. Adding +unrelated company profiles must not grow this agent’s YAML. Inherited access and +history remain in their owning tabs. An interrupted multi-resource save must report partial progress, preserve returned one-time keys, and require reload before another save. Domain APIs retain their existing authorization; a multi-resource save is not atomic. diff --git a/packages/shared/src/agent-configuration-resources.ts b/packages/shared/src/agent-configuration-resources.ts index bc407fcc18c..a71afc1c1a4 100644 --- a/packages/shared/src/agent-configuration-resources.ts +++ b/packages/shared/src/agent-configuration-resources.ts @@ -57,3 +57,33 @@ export const agentConfigurationResourceShape = { }; export type AgentConfigurationResources = z.infer>; + +const resources = agentConfigurationResourceShape; +const tools = resources.tools.unwrap().shape; +const profileReferenceSchema = tools.profiles.element.partial().extend({ + profileKey: id.optional(), +}).refine(value => value.profileId || value.profileKey || value.configuration, { + message: "Specify profileKey, profileId, or a new profile configuration.", +}).refine(value => value.profileId !== null || !value.profileKey, { + message: "A new profile (profileId: null) cannot also reference an existing profileKey.", +}); + +// Omission preserves saved values. Arrays and open configuration maps, when +// explicitly supplied, still replace their saved counterparts. +export const agentConfigurationInputResourceShape = { + permissions: resources.permissions.unwrap().partial().optional(), + instructions: resources.instructions.unwrap().partial().extend({ + files: z.record(filePath, z.string().max(1024 * 1024).nullable()).optional(), + }).optional(), + skills: resources.skills, + tools: z.object({ + connections: z.array(tools.connections.element.partial({ installed: true })).optional(), + profiles: z.array(profileReferenceSchema).optional(), + policies: tools.policies.optional(), + }).strict().optional(), + channels: z.array(resources.channels.unwrap().element.partial().required({ id: true })).optional(), + apiKeys: resources.apiKeys.unwrap().partial().optional(), + revisions: resources.revisions, + secrets: resources.secrets.unwrap().partial().optional(), + channelOperations: resources.channelOperations.unwrap().partial().optional(), +}; diff --git a/packages/shared/src/agent-configuration.ts b/packages/shared/src/agent-configuration.ts index fad56b27a9a..f6b564c0d60 100644 --- a/packages/shared/src/agent-configuration.ts +++ b/packages/shared/src/agent-configuration.ts @@ -3,7 +3,7 @@ import { resolveAgentAppearance } from "./agent-appearance.js"; import type { Agent } from "./types/agent.js"; import { createAgentSchema } from "./validators/agent.js"; import { objectWithoutDefaults } from "./validators/partial.js"; -import { agentConfigurationResourceShape } from "./agent-configuration-resources.js"; +import { agentConfigurationInputResourceShape, agentConfigurationResourceShape } from "./agent-configuration-resources.js"; export const agentConfigurationSpecSchema = objectWithoutDefaults(createAgentSchema.pick({ name: true, @@ -30,6 +30,10 @@ export const agentConfigurationDocumentSchema = z.object({ export type AgentConfigurationSpec = z.infer; export type AgentConfigurationDocument = z.infer; +export const agentConfigurationInputSchema = agentConfigurationDocumentSchema.extend({ + spec: agentConfigurationSpecSchema.partial().extend(agentConfigurationInputResourceShape), +}); +export type AgentConfigurationInput = z.infer; export type AgentConfigurationSource = Omit, "role" | "adapterType"> & { role: string; adapterType: string; diff --git a/tests/e2e/agent-configuration.spec.ts b/tests/e2e/agent-configuration.spec.ts index de4427c6739..227e67ba551 100644 --- a/tests/e2e/agent-configuration.spec.ts +++ b/tests/e2e/agent-configuration.spec.ts @@ -3,7 +3,7 @@ import os from "node:os"; import path from "node:path"; import { expect, test as base, type APIRequestContext, type APIResponse, type Page } from "@playwright/test"; import { parse, stringify } from "yaml"; -import type { AgentConfigurationDocument } from "@paperclipai/shared"; +import type { AgentConfigurationInput } from "@paperclipai/shared"; import { startReviewProvider } from "../fixtures/connection-review-provider"; const test = base.extend<{ toolProvider: Awaited>; settings: void }>({ @@ -25,8 +25,6 @@ const test = base.extend<{ toolProvider: Awaited page.getByRole("textbox", { name: "Agent YAML configuration" }); -async function document(page: Page): Promise { +async function document(page: Page): Promise { await expect(editor(page)).toBeVisible(); await expect(editor(page)).toHaveAttribute("contenteditable", "true"); let source = ""; // Keep the selection inside the editor across configuration rerenders. // Never parse a whole-page selection or a prior clipboard value as YAML. await expect.poll(async () => { - await page.evaluate(() => navigator.clipboard.writeText("")); await editor(page).click(); await editor(page).press("ControlOrMeta+a"); const selectedEditor = await editor(page).evaluate(element => { @@ -58,22 +55,27 @@ async function document(page: Page): Promise { && element.contains(selection.anchorNode) && element.contains(selection.focusNode); }); if (!selectedEditor) return false; - await page.keyboard.press("ControlOrMeta+c"); - source = await page.evaluate(() => navigator.clipboard.readText()); + // Capture the editor's copy event without sharing the OS clipboard with + // other browser tests or the person using this computer. + source = await editor(page).evaluate(element => { + const clipboardData = new DataTransfer(); + element.dispatchEvent(new ClipboardEvent("copy", { clipboardData, bubbles: true, cancelable: true })); + return clipboardData.getData("text/plain"); + }); return /^apiVersion: paperclip\/v1(?:\r?\n|$)/.test(source); }, { message: "Copy the current YAML from the focused editor" }).toBe(true); return parse(source); } -async function edit(page: Page, change: (value: AgentConfigurationDocument) => void) { +async function edit(page: Page, change: (value: AgentConfigurationInput) => void) { const value = await document(page); change(value); - await page.evaluate(source => navigator.clipboard.writeText(source), stringify(value, { lineWidth: 0 })); - await editor(page).press("ControlOrMeta+v"); + await editor(page).press("ControlOrMeta+a"); + await page.keyboard.insertText(stringify(value, { lineWidth: 0 })); } async function save(page: Page) { await page.getByRole("button", { name: "Save changes", exact: true }).click(); await expect(page.getByText("Unsaved changes", { exact: true })).toHaveCount(0); - await expect(page.getByRole("button", { name: "Save changes", exact: true })).toBeDisabled(); + await expect(page.getByRole("button", { name: "Save changes", exact: true })).toBeDisabled({ timeout: 30_000 }); await expect(page.getByRole("button", { name: "Reload saved", exact: true })).toBeEnabled(); await expect(page.getByRole("alert")).toHaveCount(0); } @@ -125,6 +127,40 @@ test("Secrets shows masked inline variables and preserves them when another vari expect(after.configurationRevision).toBe(before.configurationRevision); }); +test("compact YAML stays small as the company grows and preserves hidden configuration", async ({ page, request }) => { + const { company, agent, url } = await setup(request); + await json(await request.patch(`/api/agents/${agent.id}/instructions-bundle`, { data: { mode: "managed" } })); + const entry = await json(await request.get(`/api/agents/${agent.id}/instructions-bundle/file?path=AGENTS.md`)); + const instructions = "Keep the agent instructions outside its YAML configuration.\n".repeat(500); + await json(await request.put(`/api/agents/${agent.id}/instructions-bundle/file`, { data: { + path: "AGENTS.md", content: instructions, baseHash: entry.contentHash, baseRevisionId: entry.revision?.id ?? null, + } })); + const profiles = []; + for (let index = 0; index < 8; index++) profiles.push(await json(await request.post(`/api/companies/${company.id}/tools/profiles`, { data: { + profileKey: `profile-${index}`, name: `Profile ${index}`, defaultAction: "deny", + entries: Array.from({ length: 30 }, (_, tool) => ({ selectorType: "tool_name", effect: "include", toolName: `fixture_tool_${tool}` })), + } }))); + await json(await request.post(`/api/companies/${company.id}/tools/profiles/${profiles[0].id}/bind`, { data: { targetType: "agent", targetId: agent.id, priority: 100 } })); + await page.goto(`${url}/advanced`); + const compact = await document(page); + const source = stringify(compact); + expect(source.split("\n").length).toBeLessThan(60); + for (const omitted of ["observed:", "fixture_tool_", "profile-7", "Keep the agent instructions", "create:", "metadata: null"]) expect(source).not.toContain(omitted); + expect(compact.spec.tools?.profiles).toEqual([{ profileKey: "profile-0" }]); + expect(compact.spec.instructions).toEqual({ mode: "managed", entryFile: "AGENTS.md" }); + await edit(page, value => { value.spec = { title: "Sparse saved edit" }; }); + await save(page); + const saved = await json(await request.get(`/api/agents/${agent.id}/configuration`)); + expect(saved.name).toBe(agent.name); + expect(saved.adapterConfig).toMatchObject({ command: "true", removable: "old value" }); + expect((await json(await request.get(`/api/agents/${agent.id}/instructions-bundle/file?path=AGENTS.md`))).content).toBe(instructions); + await edit(page, value => { value.spec.tools = { profiles: [{ profileKey: "profile-0" }, { profileKey: "profile-1" }] }; }); + await save(page); + const selected = await json(await request.get(`/api/companies/${company.id}/tools/profiles`)); + expect(selected.profiles.find((profile: { id: string }) => profile.id === profiles[1].id).bindings).toContainEqual(expect.objectContaining({ targetType: "agent", targetId: agent.id })); + for (const profile of selected.profiles.filter((profile: { id: string }) => profiles.some(created => created.id === profile.id))) expect(profile.entries).toHaveLength(30); +}); + test("Advanced YAML persists settings and commands through their real resource APIs", async ({ page, request, toolProvider }, info) => { test.setTimeout(120_000); const { company, agent, url } = await setup(request); @@ -140,20 +176,20 @@ test("Advanced YAML persists settings and commands through their real resource A await edit(page, value => { value.spec.title = "Saved YAML engineer"; value.spec.budgetMonthlyCents = 12300; - delete value.spec.adapterConfig.removable; - value.spec.adapterConfig.env = { YAML_VARIABLE: { type: "plain", value: "saved variable" } }; - value.spec.instructions!.files["workflow.md"] = "Saved through YAML.\n"; + delete value.spec.adapterConfig!.removable; + value.spec.adapterConfig!.env = { YAML_VARIABLE: { type: "plain", value: "saved variable" } }; + value.spec.instructions!.files = { "workflow.md": "Saved through YAML.\n" }; value.spec.skills = [{ key: skill.key, versionId: skill.currentVersionId }]; - value.spec.tools!.connections.find(row => row.connectionId === connection.id)!.installed = true; + value.spec.tools = { connections: [{ connectionId: connection.id }] }; value.spec.tools!.profiles = [{ profileId: null, priority: 100, metadata: null, configuration: { profileKey: "yaml-profile", name: "YAML profile", description: null, status: "active", defaultAction: "deny", metadata: null, entries: [{ selectorType: "tool_name", toolName: "read_repository", effect: "include" }], } }]; value.spec.tools!.policies = [{ id: null, configuration: { name: "YAML block", policyType: "block", priority: 100, enabled: true, selectors: { agentIds: [agent.id], toolNames: ["delete_repository"] } } }]; - value.spec.permissions!.canCreateSkills = false; - value.spec.secrets!.create = [{ secret: { name: "YAML secret", value: "disposable-yaml-secret" }, bind: { env: ["YAML_SECRET"], api: [] } }]; - value.spec.apiKeys!.create = [{ name: "YAML automation", scope: { kind: "standard" } }]; - value.spec.channelOperations!.create = [{ provider: "telegram" }]; + value.spec.permissions = { canCreateSkills: false }; + value.spec.secrets = { create: [{ secret: { name: "YAML secret", value: "disposable-yaml-secret" }, bind: { env: ["YAML_SECRET"], api: [] } }] }; + value.spec.apiKeys = { create: [{ name: "YAML automation", scope: { kind: "standard" } }] }; + value.spec.channelOperations = { create: [{ provider: "telegram" }] }; }); await save(page); const token = page.getByRole("textbox", { name: "API key: YAML automation token" }); @@ -163,35 +199,45 @@ test("Advanced YAML persists settings and commands through their real resource A const saved = await document(page); expect(saved.spec).toMatchObject({ title: "Saved YAML engineer", budgetMonthlyCents: 12300, permissions: { canCreateSkills: false } }); expect(saved.spec.adapterConfig).not.toHaveProperty("removable"); - expect(saved.spec.adapterConfig.env).toMatchObject({ YAML_VARIABLE: { type: "plain", value: "***REDACTED***" }, YAML_SECRET: { type: "secret_ref" } }); + expect(saved.spec.adapterConfig!.env).toMatchObject({ YAML_VARIABLE: { type: "plain", value: "***REDACTED***" }, YAML_SECRET: { type: "secret_ref" } }); expect(stringify(saved)).not.toContain("disposable-yaml-secret"); - expect(saved.spec.instructions!.files["workflow.md"]).toBe("Saved through YAML.\n"); + expect(saved.spec.instructions).not.toHaveProperty("files"); + expect((await json(await request.get(`/api/agents/${agent.id}/instructions-bundle/file?path=workflow.md`))).content).toBe("Saved through YAML.\n"); expect(saved.spec.skills).toEqual([{ key: skill.key, versionId: skill.currentVersionId }]); - expect(saved.spec.tools!.profiles).toEqual(expect.arrayContaining([expect.objectContaining({ profileId: expect.any(String), configuration: expect.objectContaining({ name: "YAML profile", entries: [expect.objectContaining({ toolName: "read_repository" })] }) })])); + expect(saved.spec.tools!.profiles).toEqual(expect.arrayContaining([expect.objectContaining({ profileKey: "yaml-profile" })])); expect(saved.spec.tools!.policies).toEqual([expect.objectContaining({ id: expect.any(String), configuration: expect.objectContaining({ name: "YAML block" }) })]); expect(saved.spec.channels).toEqual([expect.objectContaining({ provider: "telegram" })]); - expect(saved.spec.apiKeys!.create).toEqual([]); - expect(saved.spec.secrets!.create).toEqual([]); - expect(saved.spec.channelOperations!.create).toEqual([]); + expect(saved.spec.apiKeys).toBeUndefined(); + expect(saved.spec.secrets).toBeUndefined(); + expect(saved.spec.channelOperations).toBeUndefined(); + await edit(page, value => { value.spec.skills = [skill.key]; }); + await save(page); + expect((await document(page)).spec.skills).toEqual([skill.key]); + let repeatedSkillSyncs = 0; + page.on("request", event => { + if (event.method() === "POST" && new URL(event.url()).pathname === `/api/agents/${agent.id}/skills/sync`) repeatedSkillSyncs++; + }); + await edit(page, value => { value.spec.title = "Unpinned skill stays unchanged"; }); + await save(page); + expect(repeatedSkillSyncs).toBe(0); expect((await json(await request.get(`/api/agents/${agent.id}/configuration`))).adapterConfig.env.YAML_SECRET.type).toBe("secret_ref"); expect((await json(await request.get(`/api/tool-connections/${connection.id}/installs`))).installs).toContainEqual(expect.objectContaining({ targetType: "agent", targetId: agent.id })); const key = (await json(await request.get(`/api/agents/${agent.id}/keys`))).find((row: { name: string }) => row.name === "YAML automation"); await edit(page, value => { - value.spec.apiKeys!.revoke = [key.id]; - value.spec.channels![0].settings.allowUnlinkedPeople = true; - delete value.spec.instructions!.files["workflow.md"]; + value.spec.apiKeys = { revoke: [key.id] }; + value.spec.channels![0].settings = { allowUnlinkedPeople: true }; + value.spec.instructions!.files = { "workflow.md": null }; value.spec.tools!.profiles = []; value.spec.tools!.policies = []; - value.spec.tools!.connections.find(row => row.connectionId === connection.id)!.installed = false; + value.spec.tools!.connections = []; value.spec.skills = []; }); await save(page); await page.reload(); const cleared = await document(page); - expect(cleared.spec.instructions!.files).not.toHaveProperty("workflow.md"); - expect(cleared.spec.skills).toEqual([]); - expect(cleared.spec.tools!.profiles).toEqual([]); - expect(cleared.spec.tools!.policies).toEqual([]); + expect((await json(await request.get(`/api/agents/${agent.id}/instructions-bundle`))).files.map((file: { path: string }) => file.path)).not.toContain("workflow.md"); + expect(cleared.spec.skills).toBeUndefined(); + expect(cleared.spec.tools).toBeUndefined(); expect(cleared.spec.channels![0].settings.allowUnlinkedPeople).toBe(true); expect(cleared.spec.channels![0].settings).not.toHaveProperty("communicationInstructions"); expect((await json(await request.get(`/api/agents/${agent.id}/keys`))).find((row: { id: string }) => row.id === key.id).revokedAt).not.toBeNull(); @@ -219,7 +265,7 @@ test("renaming an agent keeps newly created key tokens visible through the route await page.goto(`${url}/advanced`); await edit(page, value => { value.spec.name = "Renamed YAML engineer"; - value.spec.apiKeys!.create = [{ name: "Rename key", scope: { kind: "standard" } }]; + value.spec.apiKeys = { create: [{ name: "Rename key", scope: { kind: "standard" } }] }; }); await save(page); await expect(page).toHaveURL(/\/agents\/renamed-yaml-engineer\/advanced$/); @@ -248,7 +294,7 @@ test("a lost creation response requires reload and cannot blindly replay command await edit(page, value => { value.spec.name = "Partially renamed YAML engineer"; value.spec.title = "Saved before lost response"; - value.spec.apiKeys!.create = ["First key", "Second key"].map(name => ({ name, scope: { kind: "standard" } })); + value.spec.apiKeys = { create: ["First key", "Second key"].map(name => ({ name, scope: { kind: "standard" } })) }; }); await page.getByRole("button", { name: "Save changes", exact: true }).click(); await expect(page.getByRole("alert")).toContainText("Reload saved before saving again"); @@ -267,7 +313,7 @@ test("a lost creation response requires reload and cannot blindly replay command await page.getByRole("button", { name: "Reload saved", exact: true }).click(); await expect(page).toHaveURL(/\/agents\/partially-renamed-yaml-engineer\/advanced$/); await expect(page.getByRole("button", { name: "Reload saved", exact: true })).toBeEnabled(); - expect((await document(page)).spec.apiKeys!.create).toEqual([]); + expect((await document(page)).spec.apiKeys).toBeUndefined(); await expect(page.getByRole("textbox", { name: "API key: First key token" })).toBeVisible(); await edit(page, value => { value.spec.title = "Recovered save"; }); await save(page); @@ -283,7 +329,7 @@ test("switching instructions location loads existing destination files without o await page.goto(`${url}/advanced`); await edit(page, value => { value.spec.instructions!.mode = "external"; value.spec.instructions!.rootPath = destination; }); await save(page); - expect((await document(page)).spec.instructions!.files["AGENTS.md"]).toBe("Keep destination instructions.\n"); + expect((await document(page)).spec.instructions).not.toHaveProperty("files"); expect(await readFile(path.join(destination, "AGENTS.md"), "utf8")).toBe("Keep destination instructions.\n"); }); @@ -297,7 +343,7 @@ test("revision restore is an explicit separate save and reloads the restored con expect(revision).toBeTruthy(); await edit(page, value => { value.spec.title = "After restore target"; }); await save(page); - await edit(page, value => { value.spec.revisions!.restore = revision.id; value.spec.title = "Conflicting edit"; }); + await edit(page, value => { value.spec.revisions = { restore: revision.id }; value.spec.title = "Conflicting edit"; }); await expect(page.getByRole("alert")).toContainText("Restore a revision in its own save"); await expect(page.getByRole("button", { name: "Save changes", exact: true })).toBeDisabled(); await edit(page, value => { value.spec.title = "After restore target"; }); @@ -305,5 +351,5 @@ test("revision restore is an explicit separate save and reloads the restored con await page.reload(); const restored = await document(page); expect(restored.spec.title).toBe("Before restore"); - expect(restored.spec.revisions!.restore).toBeNull(); + expect(restored.spec.revisions).toBeUndefined(); }); diff --git a/ui/src/lib/agent-configuration-yaml.test.ts b/ui/src/lib/agent-configuration-yaml.test.ts index 0a65d0d4b11..fd085a69c1a 100644 --- a/ui/src/lib/agent-configuration-yaml.test.ts +++ b/ui/src/lib/agent-configuration-yaml.test.ts @@ -21,7 +21,7 @@ describe("agent configuration YAML", () => { it("sends only changed fields and replaces the adapter map when a key is removed", () => { const doc = parseAgentConfigurationYaml(serializeAgentConfiguration(configuration)); - delete doc.spec.adapterConfig.extra; + delete doc.spec.adapterConfig!.extra; doc.spec.budgetMonthlyCents = 20000; expect(agentConfigurationPatch(doc, configuration)).toEqual({ adapterConfig: { model: "gpt-5", env: configuration.adapterConfig.env }, @@ -37,7 +37,6 @@ describe("agent configuration YAML", () => { it.each([ ["unknown version", (text: string) => text.replace("paperclip/v1", "paperclip/v2")], ["unknown field", (text: string) => `${text} typo: true\n`], - ["missing field", (text: string) => text.replace(" name: Codie\n", "")], ["invalid budget", (text: string) => text.replace("budgetMonthlyCents: 12000", "budgetMonthlyCents: -1")], ["invalid environment", (text: string) => text.replace("defaultEnvironmentId: null", "defaultEnvironmentId: unknown")], ["duplicate key", (text: string) => `${text} name: duplicate\n`], @@ -46,7 +45,7 @@ describe("agent configuration YAML", () => { ["NaN", (text: string) => text.replace("metadata: null", "metadata: {value: .nan}")], ["merge key", (text: string) => text.replace("metadata: null", "metadata: {<<: {name: Other}}")], ["reserved key", (text: string) => text.replace("metadata: null", "metadata: {__proto__: {bad: true}}")], - ["permission mutation", (text: string) => `${text} permissions: {canCreateAgents: true}\n`], + ["invalid permission", (text: string) => `${text} permissions: {canCreateAgents: wrong}\n`], ["status mutation", (text: string) => `${text} status: idle\n`], ])("rejects %s before saving", (_name, modify) => { expect(() => parseAgentConfigurationYaml(modify(serializeAgentConfiguration(configuration)))).toThrow(); diff --git a/ui/src/lib/agent-configuration-yaml.ts b/ui/src/lib/agent-configuration-yaml.ts index 9311a322233..703e8af0eb5 100644 --- a/ui/src/lib/agent-configuration-yaml.ts +++ b/ui/src/lib/agent-configuration-yaml.ts @@ -1,9 +1,10 @@ import { isAlias, parseDocument, stringify, visit } from "yaml"; import { - agentConfigurationDocumentSchema, + agentConfigurationInputSchema, agentConfigurationSpec, canonicalAgentConfiguration, type AgentConfigurationDocument, + type AgentConfigurationInput, type AgentConfigurationSource, } from "@paperclipai/shared"; @@ -11,7 +12,7 @@ export function serializeAgentConfiguration(agent: AgentConfigurationSource): st return serializeAgentConfigurationDocument({ apiVersion: "paperclip/v1", kind: "Agent", spec: agentConfigurationSpec(agent) } as AgentConfigurationDocument); } -export function serializeAgentConfigurationDocument(document: AgentConfigurationDocument): string { +export function serializeAgentConfigurationDocument(document: AgentConfigurationDocument | AgentConfigurationInput): string { return stringify(document, { lineWidth: 0, aliasDuplicateObjects: false, }); @@ -34,7 +35,7 @@ function assertJson(value: unknown, depth = 0): void { throw new Error("Configuration must contain only JSON-compatible values (no NaN or infinity)."); } -export function parseAgentConfigurationYaml(source: string): AgentConfigurationDocument { +export function parseAgentConfigurationYaml(source: string): AgentConfigurationInput { if (source.length > 8 * 1024 * 1024 || new TextEncoder().encode(source).byteLength > 8 * 1024 * 1024) throw new Error("Configuration must be smaller than 8 MiB."); const document = parseDocument(source, { version: "1.2", schema: "core", uniqueKeys: true, stringKeys: true }); const problem = document.errors[0] ?? document.warnings[0]; @@ -47,17 +48,18 @@ export function parseAgentConfigurationYaml(source: string): AgentConfigurationD }); const value: unknown = document.toJS({ maxAliasCount: 0 }); assertJson(value); - const parsed = agentConfigurationDocumentSchema.safeParse(value); + const parsed = agentConfigurationInputSchema.safeParse(value); if (!parsed.success) { throw new Error(parsed.error.issues.map(issue => `${issue.path.join(".") || "document"}: ${issue.message}`).join("\n")); } return parsed.data; } -export function agentConfigurationPatch(document: AgentConfigurationDocument, saved: AgentConfigurationSource) { +export function agentConfigurationPatch(document: AgentConfigurationInput, saved: AgentConfigurationSource) { const before = agentConfigurationSpec(saved); const patch: Record = {}; for (const key of Object.keys(before) as Array) { + if (!Object.hasOwn(document.spec, key)) continue; const value = document.spec[key]; if (canonicalAgentConfiguration(value) !== canonicalAgentConfiguration(before[key as keyof typeof before])) { patch[key] = value; diff --git a/ui/src/lib/agent-configuration/channels.ts b/ui/src/lib/agent-configuration/channels.ts index b599b9a52da..01711893939 100644 --- a/ui/src/lib/agent-configuration/channels.ts +++ b/ui/src/lib/agent-configuration/channels.ts @@ -23,8 +23,8 @@ export function planChannelChanges(snapshot: AgentConfigurationSnapshot, documen !equal(value, previous.settings[key as keyof typeof previous.settings]))); add(`Channel settings: ${row.id}`, () => chatEndpointsApi.update(row.id, changes)); } - if (!equal(previous.resources, row.resources)) { - const resources = previous.resources.map(resource => row.resources.find(value => value.id === resource.id) ?? { ...resource, enabled: false }); + const resources = previous.resources.map(resource => row.resources.find(value => value.id === resource.id) ?? { ...resource, enabled: false }); + if (!equal(previous.resources, resources)) { add(`Channel resources: ${row.id}`, () => chatEndpointsApi.updateResources(row.id, resources)); } } diff --git a/ui/src/lib/agent-configuration/compact.ts b/ui/src/lib/agent-configuration/compact.ts new file mode 100644 index 00000000000..02ab186d7bd --- /dev/null +++ b/ui/src/lib/agent-configuration/compact.ts @@ -0,0 +1,66 @@ +import type { AgentConfigurationDocument, AgentConfigurationInput } from "@paperclipai/shared"; + +const nonempty = (value: unknown) => value !== null && value !== undefined + && (typeof value !== "object" || Object.keys(value).length > 0); + +export function compactAgentConfiguration(document: AgentConfigurationDocument): AgentConfigurationInput { + const spec: AgentConfigurationInput["spec"] = { ...document.spec }; + if (spec.adapterConfig && Object.hasOwn(spec.adapterConfig, "promptTemplate")) { + spec.adapterConfig = { ...spec.adapterConfig }; + delete spec.adapterConfig.promptTemplate; + } + for (const key of ["title", "icon", "reportsTo", "capabilities", "defaultEnvironmentId", "metadata", "adapterConfig", "runtimeConfig"] as const) { + if (!nonempty(spec[key])) delete spec[key]; + } + if (spec.budgetMonthlyCents === 0) delete spec.budgetMonthlyCents; + if (spec.permissions) { + const defaults = { canCreateAgents: false, canCreateSkills: true, canAssignTasks: false, trustPreset: "standard" }; + spec.permissions = Object.fromEntries(Object.entries(spec.permissions).filter(([key, value]) => + value !== defaults[key as keyof typeof defaults] && nonempty(value))); + if (!Object.keys(spec.permissions).length) delete spec.permissions; + } + if (spec.instructions?.mode) { + spec.instructions = { + mode: spec.instructions.mode, + ...(spec.instructions.mode === "external" ? { rootPath: spec.instructions.rootPath } : {}), + entryFile: spec.instructions.entryFile, + }; + } else if (document.spec.adapterConfig.promptTemplate && spec.instructions) { + spec.instructions = { entryFile: spec.instructions.entryFile }; + } else delete spec.instructions; + if (spec.skills?.length) spec.skills = spec.skills.map(skill => typeof skill === "string" || skill.versionId ? skill : skill.key); + else delete spec.skills; + const tools = document.spec.tools; + if (tools) { + spec.tools = { + ...(tools.connections.some(row => row.installed) ? { + connections: tools.connections.filter(row => row.installed).map(row => ({ connectionId: row.connectionId })), + } : {}), + ...(tools.profiles.length ? { profiles: tools.profiles.map(row => ({ + ...(row.configuration.profileKey ? { profileKey: row.configuration.profileKey } : { profileId: row.profileId }), + ...(row.priority !== 100 ? { priority: row.priority } : {}), + ...(nonempty(row.metadata) ? { metadata: row.metadata } : {}), + })) } : {}), + ...(tools.policies.length ? { policies: tools.policies } : {}), + }; + if (!Object.keys(spec.tools).length) delete spec.tools; + } + if (spec.channels?.length) spec.channels = spec.channels.map(row => { + const settings = Object.fromEntries(Object.entries(row.settings ?? {}).filter(([, value]) => nonempty(value) && value !== "")); + const resources = row.resources?.filter(resource => resource.enabled).map(resource => ({ + id: resource.id, enabled: true, + ...(resource.executionDefaults ? { executionDefaults: resource.executionDefaults } : {}), + })); + return { id: row.id, provider: row.provider, + ...(Object.keys(settings).length ? { settings } : {}), ...(resources?.length ? { resources } : {}), + }; + }); + else delete spec.channels; + for (const key of ["apiKeys", "secrets", "channelOperations", "revisions"] as const) { + if (!spec[key]) continue; + const operations = Object.fromEntries(Object.entries(spec[key]).filter(([, value]) => nonempty(value))); + if (Object.keys(operations).length) Object.assign(spec, { [key]: operations }); + else delete spec[key]; + } + return { apiVersion: document.apiVersion, kind: document.kind, spec }; +} diff --git a/ui/src/lib/agent-configuration/load.ts b/ui/src/lib/agent-configuration/load.ts index 833ff130eef..5d13f24800c 100644 --- a/ui/src/lib/agent-configuration/load.ts +++ b/ui/src/lib/agent-configuration/load.ts @@ -77,10 +77,12 @@ export async function loadAgentConfiguration(id: string, companyId: string) { }, instructions: { mode: bundle.mode, rootPath: bundle.rootPath, entryFile: bundle.entryFile, files: Object.fromEntries(files.map(file => [file.path, file.content])) }, - skills: skills.desiredSkillEntries ?? skills.desiredSkills.map(key => ({ key, versionId: null })), + skills: (skills.desiredSkillEntries ?? skills.desiredSkills.map(key => ({ key, versionId: null }))) + .map(skill => ({ ...skill, versionId: skill.versionId ?? null })), tools: { - connections: connections.connections.map(connection => ({ connectionId: connection.id, - installed: (connection.installs ?? []).some(install => install.targetType === "agent" && install.targetId === id) })), + connections: connections.connections.flatMap(connection => + (connection.installs ?? []).some(install => install.targetType === "agent" && install.targetId === id) + ? [{ connectionId: connection.id, installed: true }] : []), profiles: directProfiles, policies: directPolicies.map(policy => ({ id: policy.id, configuration: policyConfiguration(policy) })), }, @@ -98,19 +100,6 @@ export async function loadAgentConfiguration(id: string, companyId: string) { secrets: { create: [], rotate: [], update: [], proposals: [] }, channelOperations: { create: [], setup: [], revokeIdentityLinks: [] }, }, - observed: { - apiKeys: keys, revisions: revisions.map(({ id, source, changedKeys, createdAt }) => ({ id, source, changedKeys, createdAt })), - instructionFiles: bundle.files.filter(file => !file.editable || file.binary || file.virtual), instructionWarnings: bundle.warnings, - skillWarnings: skills.warnings, - connections: connections.connections.map(connection => ({ id: connection.id, name: connection.name, - inherited: (connection.installs ?? []).some(install => install.targetType === "company" && install.targetId === companyId) })), - availableProfiles: profiles.profiles.map(profile => ({ id: profile.id, configuration: profileConfiguration(profile) })), - inheritedProfiles: profiles.profiles.filter(profile => profile.bindings.some(binding => binding.targetType === "company" && binding.targetId === companyId)).map(profile => ({ id: profile.id, configuration: profileConfiguration(profile) })), - inheritedPolicies: policies.policies.filter(policy => !directPolicies.includes(policy) && (policyAgentIds(policy).length === 0 || policyAgentIds(policy).includes(id))).map(policy => ({ id: policy.id, configuration: policyConfiguration(policy) })), - channels: channels.map(({ detail, resources, principals }) => ({ id: detail.id, status: detail.status, botLabel: detail.botLabel, resources, principals })), - secrets: secrets.map(({ id, name, key, provider, latestVersion }) => ({ id, name, key, provider, latestVersion })), - secretProposals: proposals.filter(proposal => (proposal.target?.id === id || proposal.proposedBy.id === id)), - }, }) as AgentConfigurationDocument; return { agent, core, bundle, files: new Map(files.map(file => [file.path, file])), connections: connections.connections, profiles: profiles.profiles, policies: policies.policies, channels, keys, revisions, secrets, proposals, document }; diff --git a/ui/src/lib/agent-configuration/resolve.ts b/ui/src/lib/agent-configuration/resolve.ts new file mode 100644 index 00000000000..39aff201d67 --- /dev/null +++ b/ui/src/lib/agent-configuration/resolve.ts @@ -0,0 +1,61 @@ +import { agentConfigurationDocumentSchema, agentConfigurationInputSchema, type AgentConfigurationInput } from "@paperclipai/shared"; +import { equal, profileConfiguration, type AgentConfigurationSnapshot } from "./load"; +import { requireItem } from "./steps"; + +export function resolveAgentConfiguration(snapshot: AgentConfigurationSnapshot, input: AgentConfigurationInput) { + const document = agentConfigurationInputSchema.parse(input); + if (document.observed !== undefined && !equal(document.observed, snapshot.document.observed)) { + throw new Error("observed is read-only. Remove this block and edit spec to change configuration."); + } + const before = snapshot.document.spec; + const spec = { ...before, ...document.spec }; + // Legacy instruction text is omitted from the adapter display, but remains + // editable explicitly or through Instructions. Other adapter keys retain + // replacement semantics. + if (document.spec.adapterConfig && !Object.hasOwn(document.spec.adapterConfig, "promptTemplate") && Object.hasOwn(before.adapterConfig, "promptTemplate")) { + spec.adapterConfig = { ...document.spec.adapterConfig, promptTemplate: before.adapterConfig.promptTemplate }; + } + for (const key of ["permissions", "instructions", "tools", "apiKeys", "secrets", "channelOperations", "revisions"] as const) { + if (document.spec[key]) Object.assign(spec, { [key]: { ...before[key], ...document.spec[key] } }); + } + if (document.spec.skills) spec.skills = document.spec.skills.map(skill => + typeof skill === "string" ? { key: skill, versionId: null } : { ...skill, versionId: skill.versionId ?? null }); + if (document.spec.instructions?.files) { + const files = { ...before.instructions?.files }; + for (const [path, content] of Object.entries(document.spec.instructions.files)) { + const existing = snapshot.bundle.files.find(file => file.path === path); + if (existing && (!existing.editable || existing.binary || existing.virtual)) { + throw new Error(`Instruction file ${path} cannot be edited as text.`); + } + if (content === null) delete files[path]; + else files[path] = content; + } + spec.instructions = { ...spec.instructions, files }; + } + if (document.spec.tools?.connections) spec.tools!.connections = document.spec.tools.connections.map(row => ({ ...row, installed: row.installed ?? true })); + if (document.spec.tools?.profiles) spec.tools!.profiles = document.spec.tools.profiles.map(row => { + const existing = row.profileId || row.profileKey + ? requireItem(snapshot.profiles, profile => (!row.profileId || profile.id === row.profileId) + && (!row.profileKey || profile.profileKey === row.profileKey), `Profile ${row.profileKey ?? row.profileId}`) + : null; + const binding = before.tools?.profiles.find(profile => profile.profileId === existing?.id); + return { + profileId: existing?.id ?? null, + updateSharedDefinition: row.updateSharedDefinition ?? false, + priority: row.priority ?? binding?.priority ?? 100, + metadata: row.metadata === undefined ? binding?.metadata ?? null : row.metadata, + configuration: row.configuration ?? (existing ? profileConfiguration(existing) : undefined), + }; + }); + if (document.spec.channels) spec.channels = document.spec.channels.map(row => { + const previous = requireItem(before.channels ?? [], channel => channel.id === row.id, `Channel ${row.id}`); + return { + ...previous, ...row, + settings: { ...previous.settings, ...row.settings }, + resources: row.resources?.map(resource => ({ + ...requireItem(previous.resources, existing => existing.id === resource.id, `Channel resource ${resource.id}`), ...resource, + })) ?? previous.resources, + }; + }); + return agentConfigurationDocumentSchema.parse({ ...document, spec, observed: snapshot.document.observed }); +} diff --git a/ui/src/lib/agent-configuration/save.test.ts b/ui/src/lib/agent-configuration/save.test.ts index e497d0b9c0b..9383c39fe58 100644 --- a/ui/src/lib/agent-configuration/save.test.ts +++ b/ui/src/lib/agent-configuration/save.test.ts @@ -1,8 +1,9 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { appearanceForPalette, type AgentConfigurationDocument } from "@paperclipai/shared"; +import { appearanceForPalette, type AgentConfigurationDocument, type AgentConfigurationInput } from "@paperclipai/shared"; import { parseAgentConfigurationYaml, serializeAgentConfigurationDocument } from "../agent-configuration-yaml"; import { loadAgentConfiguration } from "./load"; import { ConfigurationSaveInterrupted, planAgentConfiguration, saveAgentConfiguration } from "./save"; +import { compactAgentConfiguration } from "./compact"; const api = vi.hoisted(() => ({ agents: Object.fromEntries(["getConfiguration", "get", "instructionsBundle", "instructionsFile", "skills", "listKeys", "listConfigRevisions", "update", "updateInstructionsBundle", "saveInstructionsFile", "deleteInstructionsFile", "syncSkills", "updatePermissions", "createKey", "revokeKey", "rollbackConfigRevision"].map(key => [key, vi.fn()])), @@ -41,7 +42,7 @@ beforeEach(() => { api.agents.skills.mockResolvedValue({ desiredSkills: ["paperclip"], desiredSkillEntries: [{ key: "paperclip", versionId: resourceId }], warnings: [] }); api.agents.listKeys.mockResolvedValue([{ id: "key", name: "Previous", scope: { kind: "standard" }, revokedAt: null }]); api.agents.listConfigRevisions.mockResolvedValue([{ id: "revision", source: "board", changedKeys: ["name"] }]); - api.tools.listConnections.mockResolvedValue({ connections: [connection] }); + api.tools.listConnections.mockResolvedValue({ connections: [copy(connection)] }); api.tools.listProfiles.mockResolvedValue({ profiles: [] }); api.tools.listPolicies.mockResolvedValue({ policies: [] }); api.tools.getConnectionInstalls.mockResolvedValue({ installs }); api.tools.createProfile.mockResolvedValue({ id: "new-profile" }); @@ -57,10 +58,10 @@ describe("complete agent configuration", () => { it("loads every resource section and round trips to a no-op without credential values", async () => { const saved = await snapshot(); const doc = roundTrip(saved.document); - expect(doc.spec.instructions?.files["AGENTS.md"]).toBe(file.content); + expect(doc.spec.instructions?.files?.["AGENTS.md"]).toBe(file.content); expect(doc.spec.skills).toEqual([{ key: "paperclip", versionId: resourceId }]); expect(doc.spec.adapterConfig).toEqual({ command: "echo" }); - expect(doc.observed?.secretProposals).toHaveLength(1); + expect(doc.observed).toBeUndefined(); expect(planAgentConfiguration(saved, doc)).toEqual([]); await saveAgentConfiguration(saved, doc, receipt, progress); expect(api.agents.getConfiguration).toHaveBeenCalledTimes(1); @@ -168,12 +169,13 @@ describe("complete agent configuration", () => { expect(receipt).toHaveBeenCalledWith({ label: "API key: Automation", id: "new-key", token: "one-time-token" }); }); - it("requires explicit clears for nested PATCH fields", async () => { + it("preserves omitted fields inside resource updates", async () => { const saved = await snapshot(); const doc = copy(saved.document); doc.spec.title = "Engineer"; delete doc.spec.channels![0].resources[0].enabled; - await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow(".enabled explicit"); - expect(api.agents.update).not.toHaveBeenCalled(); + await saveAgentConfiguration(saved, doc, receipt, progress); + expect(api.channels.updateResources).not.toHaveBeenCalled(); + expect(api.agents.update).toHaveBeenCalledWith(agentId, expect.objectContaining({ title: "Engineer" }), "company"); }); it("requires explicit consent for shared profile changes and still supports binding without changing them", async () => { @@ -293,7 +295,7 @@ describe("complete agent configuration", () => { it("rejects edits to observed context and changes to organization-wide installs", async () => { const saved = await snapshot(); const doc = copy(saved.document); - doc.observed!.apiKeys = []; + doc.observed = { apiKeys: [] }; expect(() => planAgentConfiguration(saved, doc)).toThrow("observed is read-only"); doc.observed = saved.document.observed; saved.connections[0].installs = [{ targetType: "company", targetId: "company" }] as typeof saved.connections[0]["installs"]; @@ -313,3 +315,168 @@ describe("complete agent configuration", () => { expect(api.channels.setup).toHaveBeenCalledWith("channel", { action: "pause" }); }); }); + +describe("compact agent configuration", () => { + const input = (spec: AgentConfigurationInput["spec"]): AgentConfigurationInput => ({ apiVersion: "paperclip/v1", kind: "Agent", spec }); + const profile = (id = "profile", bound = true) => ({ + id, profileKey: id, name: id, description: null, status: "active", defaultAction: "deny", metadata: null, + bindings: bound ? [{ targetType: "agent", targetId: agentId, priority: 100, metadata: null }] : [], + entries: [{ selectorType: "tool_name", effect: "include", toolName: "get_repository", applicationId: null, + connectionId: null, catalogEntryId: null, riskLevel: null, conditions: null }], + }); + + it("keeps a large company catalog and instruction contents out of the default YAML", async () => { + const profiles = Array.from({ length: 30 }, (_, i) => ({ ...profile(`profile-${i}`, i === 0), + entries: Array.from({ length: 50 }, (_, j) => ({ ...profile().entries[0], toolName: `tool_${j}` })), + })); + api.tools.listProfiles.mockResolvedValue({ profiles }); + api.agents.instructionsFile.mockResolvedValue({ ...file, content: "Do not embed this instruction file.\n".repeat(1000) }); + api.tools.listConnections.mockResolvedValue({ connections: [connection, { id: "unused", installs: [] }] }); + const saved = await snapshot(); + const compact = compactAgentConfiguration(saved.document); + const source = serializeAgentConfigurationDocument(compact); + const previous = serializeAgentConfigurationDocument({ ...saved.document, observed: { availableProfiles: profiles, inheritedProfiles: profiles } }); + expect(source.length).toBeLessThan(previous.length / 20); + expect(source.split("\n").length).toBeLessThan(70); + expect(compact.spec.tools?.profiles).toEqual([{ profileKey: "profile-0" }]); + expect(compact.spec.tools?.connections).toEqual([{ connectionId: "connection" }]); + for (const text of ["observed:", "tool_49", "profile-29", "Do not embed", "unused", "rootPath:", "create:", "revoke:", "metadata: null", "title: null"]) expect(source).not.toContain(text); + expect(compact.spec.instructions).toEqual({ mode: "managed", entryFile: "AGENTS.md" }); + expect(planAgentConfiguration(saved, parseAgentConfigurationYaml(source))).toEqual([]); + }); + + it("preserves all hidden values on sparse saves and accepts removing legacy observed metadata", async () => { + const saved = await snapshot(); + await saveAgentConfiguration(saved, input({ title: "Engineer" }), receipt, progress); + expect(api.agents.update).toHaveBeenCalledWith(agentId, { title: "Engineer", expectedConfigurationRevision: agent.configurationRevision }, "company"); + for (const method of [api.agents.saveInstructionsFile, api.agents.deleteInstructionsFile, api.agents.syncSkills, api.tools.putConnectionInstalls, api.channels.updateResources, api.agents.updatePermissions]) expect(method).not.toHaveBeenCalled(); + saved.document.observed = { oldReadOnlyCatalog: [] }; + expect(planAgentConfiguration(saved, input({}))).toEqual([]); + }); + + it("round trips unpinned skill shorthand without synchronizing unchanged skills", async () => { + api.agents.skills.mockResolvedValue({ desiredSkills: ["paperclip"], desiredSkillEntries: [{ key: "paperclip", versionId: null }], warnings: [] }); + const saved = await snapshot(); + const compact = compactAgentConfiguration(saved.document); + expect(compact.spec.skills).toEqual(["paperclip"]); + expect(planAgentConfiguration(saved, compact)).toEqual([]); + compact.spec.title = "Engineer"; + await saveAgentConfiguration(saved, compact, receipt, progress); + expect(api.agents.syncSkills).not.toHaveBeenCalled(); + }); + + it("restores a revision with unpinned skills and mixed enabled/disabled channel resources", async () => { + api.agents.skills.mockResolvedValue({ desiredSkills: ["paperclip"], desiredSkillEntries: [{ key: "paperclip", versionId: null }], warnings: [] }); + api.channels.listResources.mockResolvedValue([{ id: resourceId, enabled: true }, { id: agentId, enabled: false }]); + const saved = await snapshot(); + const compact = compactAgentConfiguration(saved.document); + expect(compact.spec.channels![0].resources).toHaveLength(1); + expect(planAgentConfiguration(saved, compact)).toEqual([]); + compact.spec.revisions = { restore: "revision" }; + await saveAgentConfiguration(saved, compact, receipt, progress); + expect(api.agents.rollbackConfigRevision).toHaveBeenCalledWith(agentId, "revision", "company"); + expect(api.agents.syncSkills).not.toHaveBeenCalled(); + expect(api.channels.updateResources).not.toHaveBeenCalled(); + }); + + it("binds a profile by key without copying or rewriting its definition", async () => { + api.tools.listProfiles.mockResolvedValue({ profiles: [profile("read", false)] }); + const saved = await snapshot(); + await saveAgentConfiguration(saved, input({ tools: { profiles: [{ profileKey: "read" }] } }), receipt, progress); + expect(api.tools.bindProfile).toHaveBeenCalledWith("company", "read", { targetType: "agent", targetId: agentId, priority: 100, metadata: null }); + expect(api.tools.updateProfile).not.toHaveBeenCalled(); + expect(api.tools.createProfile).not.toHaveBeenCalled(); + expect(api.tools.putConnectionInstalls).not.toHaveBeenCalled(); + }); + + it("changes binding priority independently of its shared definition", async () => { + api.tools.listProfiles.mockResolvedValue({ profiles: [profile()] }); + const saved = await snapshot(); + await saveAgentConfiguration(saved, input({ tools: { profiles: [{ profileKey: "profile", priority: 50 }] } }), receipt, progress); + expect(api.tools.bindProfile).toHaveBeenCalledWith("company", "profile", expect.objectContaining({ priority: 50 })); + expect(api.tools.updateProfile).not.toHaveBeenCalled(); + }); + + it("validates references before applying any core changes", async () => { + const saved = await snapshot(); + await expect(saveAgentConfiguration(saved, input({ title: "Engineer", tools: { profiles: [{ profileKey: "unknown" }] } }), receipt, progress)).rejects.toThrow("Profile unknown"); + expect(api.agents.update).not.toHaveBeenCalled(); + }); + + it("installs an unselected connection without serializing the available catalog", async () => { + api.tools.listConnections.mockResolvedValue({ connections: [{ ...connection, installs: [] }] }); + api.tools.getConnectionInstalls.mockResolvedValue({ installs: [] }); + const saved = await snapshot(); + expect(saved.document.spec.tools!.connections).toEqual([]); + await saveAgentConfiguration(saved, input({ tools: { connections: [{ connectionId: "connection" }] } }), receipt, progress); + expect(api.tools.putConnectionInstalls).toHaveBeenCalledWith("connection", [{ targetType: "agent", targetId: agentId }]); + }); + + it("ignores unrelated catalog additions during a profile binding edit", async () => { + api.tools.listProfiles.mockResolvedValue({ profiles: [profile("read", false)] }); + const saved = await snapshot(); + api.tools.listConnections.mockResolvedValue({ connections: [copy(connection), { id: "unrelated", installs: [] }] }); + await saveAgentConfiguration(saved, input({ tools: { profiles: [{ profileKey: "read" }] } }), receipt, progress); + expect(api.tools.bindProfile).toHaveBeenCalled(); + expect(api.tools.putConnectionInstalls).not.toHaveBeenCalled(); + }); + + it("requires matching IDs and keys when both are supplied", async () => { + api.tools.listProfiles.mockResolvedValue({ profiles: [profile()] }); + const saved = await snapshot(); + expect(() => planAgentConfiguration(saved, input({ tools: { profiles: [{ profileId: "profile", profileKey: "other" }] } }))).toThrow("Profile other"); + expect(() => planAgentConfiguration(saved, input({ tools: { profiles: [{ profileId: null, profileKey: "profile" }] } }))).toThrow("cannot also reference"); + }); + + it("keeps explicit empty lists and nulls as real clears", async () => { + api.agents.getConfiguration.mockResolvedValue({ ...agent, title: "Engineer" }); + api.tools.listProfiles.mockResolvedValue({ profiles: [profile()] }); + const saved = await snapshot(); + await saveAgentConfiguration(saved, input({ title: null, skills: [], tools: { connections: [], profiles: [] } }), receipt, progress); + expect(api.agents.update).toHaveBeenCalledWith(agentId, expect.objectContaining({ title: null }), "company"); + expect(api.agents.syncSkills).toHaveBeenCalledWith(agentId, [], "replace", "company"); + expect(api.tools.unbindProfile).toHaveBeenCalledWith("company", "profile", { targetType: "agent", targetId: agentId }); + expect(api.tools.putConnectionInstalls).toHaveBeenCalledWith("connection", [{ targetType: "agent", targetId: "another-agent" }]); + expect(api.agents.deleteInstructionsFile).not.toHaveBeenCalled(); + }); + + it("preserves unmentioned permission fields and channel resources", async () => { + const saved = await snapshot(); + await saveAgentConfiguration(saved, input({ permissions: { canCreateSkills: false }, channels: [{ id: "channel", settings: { allowUnlinkedPeople: true } }] }), receipt, progress); + expect(api.agents.updatePermissions).toHaveBeenCalledWith(agentId, expect.objectContaining({ canCreateAgents: false, canAssignTasks: false, canCreateSkills: false, trustPreset: "standard" }), "company"); + expect(api.channels.update).toHaveBeenCalledWith("channel", { allowUnlinkedPeople: true }); + expect(api.channels.updateResources).not.toHaveBeenCalled(); + }); + + it("edits individual instruction files and deletes only explicit null entries", async () => { + api.agents.instructionsBundle.mockResolvedValue({ mode: "managed", rootPath: "/managed/codie", entryFile: "AGENTS.md", editable: true, files: [file, { ...file, path: "old.md" }], warnings: [] }); + api.agents.instructionsFile.mockImplementation(async (_id, path) => ({ ...file, path })); + const saved = await snapshot(); + await saveAgentConfiguration(saved, input({ instructions: { files: { "new.md": "New workflow", "old.md": null } } }), receipt, progress); + expect(api.agents.saveInstructionsFile).toHaveBeenCalledWith(agentId, { path: "new.md", content: "New workflow", baseHash: null, baseRevisionId: null }, "company"); + expect(api.agents.deleteInstructionsFile).toHaveBeenCalledExactlyOnceWith(agentId, "old.md", "company", "hash"); + expect(() => planAgentConfiguration(saved, input({ instructions: { files: { "AGENTS.md": null } } }))).toThrow("entry file cannot be removed"); + }); + + it("hides legacy instruction text without deleting it during adapter edits", async () => { + const adapterConfig = { ...agent.adapterConfig, promptTemplate: "Long legacy instructions\n".repeat(1000) }; + api.agents.getConfiguration.mockResolvedValue({ ...agent, adapterConfig }); + const saved = await snapshot(); + const compact = compactAgentConfiguration(saved.document); + expect(serializeAgentConfigurationDocument(compact)).not.toContain("Long legacy instructions"); + expect(planAgentConfiguration(saved, compact)).toEqual([]); + await saveAgentConfiguration(saved, input({ adapterConfig: { command: "printf" } }), receipt, progress); + expect(api.agents.update).toHaveBeenCalledWith(agentId, expect.objectContaining({ adapterConfig: { ...adapterConfig, command: "printf" } }), "company"); + await saveAgentConfiguration(saved, input({ adapterConfig: { command: "echo", promptTemplate: "" } }), receipt, progress); + expect(api.agents.update).toHaveBeenLastCalledWith(agentId, expect.objectContaining({ adapterConfig: { ...adapterConfig, promptTemplate: "" } }), "company"); + }); + + it("rejects explicit deletion of read-only instruction files before any writes", async () => { + api.agents.instructionsBundle.mockResolvedValue({ mode: "managed", rootPath: "/managed/codie", entryFile: "AGENTS.md", editable: true, + files: [file, { ...file, path: "asset.png", editable: false, binary: true }], warnings: [], + }); + const saved = await snapshot(); + await expect(saveAgentConfiguration(saved, input({ title: "Engineer", instructions: { files: { "asset.png": null } } }), receipt, progress)).rejects.toThrow("cannot be edited as text"); + expect(api.agents.update).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/src/lib/agent-configuration/save.ts b/ui/src/lib/agent-configuration/save.ts index f60bce0490b..dd1977699cf 100644 --- a/ui/src/lib/agent-configuration/save.ts +++ b/ui/src/lib/agent-configuration/save.ts @@ -1,4 +1,4 @@ -import { agentConfigurationDocumentSchema, type AgentConfigurationDocument, type AgentConfigurationWarning } from "@paperclipai/shared"; +import type { AgentConfigurationDocument, AgentConfigurationInput, AgentConfigurationWarning } from "@paperclipai/shared"; import { agentsApi } from "../../api/agents"; import { secretsApi } from "../../api/secrets"; import { agentConfigurationPatch } from "../agent-configuration-yaml"; @@ -6,6 +6,7 @@ import { equal, loadAgentConfiguration, ownedAdapterKeys, type AgentConfiguratio import { planToolChanges } from "./tools"; import { planChannelChanges } from "./channels"; import { requireItem, unique, type ConfigurationStep, type AddConfigurationStep } from "./steps"; +import { resolveAgentConfiguration } from "./resolve"; export interface ConfigurationReceipt { label: string; id: string; token?: string } export class ConfigurationSaveInterrupted extends Error { @@ -108,7 +109,11 @@ function planSecrets(snapshot: AgentConfigurationSnapshot, document: AgentConfig } } -export function planAgentConfiguration(snapshot: AgentConfigurationSnapshot, document: AgentConfigurationDocument, receipt: (value: ConfigurationReceipt) => void = () => {}) { +export function planAgentConfiguration(snapshot: AgentConfigurationSnapshot, input: AgentConfigurationInput, receipt: (value: ConfigurationReceipt) => void = () => {}) { + return planResolvedConfiguration(snapshot, resolveAgentConfiguration(snapshot, input), receipt); +} + +function planResolvedConfiguration(snapshot: AgentConfigurationSnapshot, document: AgentConfigurationDocument, receipt: (value: ConfigurationReceipt) => void) { const steps: ConfigurationStep[] = []; const add: AddConfigurationStep = (label, apply) => { steps.push({ label, apply }); }; if (!equal(document.observed, snapshot.document.observed)) throw new Error("observed is read-only. Edit spec to change configuration."); @@ -157,10 +162,10 @@ export function planAgentConfiguration(snapshot: AgentConfigurationSnapshot, doc return steps; } -export async function saveAgentConfiguration(snapshot: AgentConfigurationSnapshot, input: AgentConfigurationDocument, +export async function saveAgentConfiguration(snapshot: AgentConfigurationSnapshot, input: AgentConfigurationInput, onReceipt: (receipt: ConfigurationReceipt) => void, onProgress: (label: string) => void) { - const document = agentConfigurationDocumentSchema.parse(input); - const initialPlan = planAgentConfiguration(snapshot, document); + const document = resolveAgentConfiguration(snapshot, input); + const initialPlan = planResolvedConfiguration(snapshot, document, onReceipt); if (!initialPlan.length) return snapshot; const current = await loadAgentConfiguration(snapshot.agent.id, snapshot.agent.companyId); if (current.core.configurationRevision !== snapshot.core.configurationRevision) throw new Error("Agent configuration changed while you were editing. Reload saved before saving."); @@ -185,7 +190,7 @@ export async function saveAgentConfiguration(snapshot: AgentConfigurationSnapsho const fresh = current.profiles.find(value => value.id === profile.profileId); if (!equal(before, fresh)) throw new Error(`Tool profile ${profile.profileId} changed while you were editing. Reload saved before saving.`); } - const steps = planAgentConfiguration(current, { ...document, spec, observed: current.document.observed }, onReceipt); + const steps = planResolvedConfiguration(current, { ...document, spec, observed: current.document.observed }, onReceipt); const completed: string[] = []; const warnings: AgentConfigurationWarning[] = []; for (const step of steps) { diff --git a/ui/src/lib/agent-configuration/tools.ts b/ui/src/lib/agent-configuration/tools.ts index 0e12102e360..7b6423fb432 100644 --- a/ui/src/lib/agent-configuration/tools.ts +++ b/ui/src/lib/agent-configuration/tools.ts @@ -12,7 +12,9 @@ export function planToolChanges(snapshot: AgentConfigurationSnapshot, document: unique(next.profiles.flatMap(row => row.profileId ? [row.profileId] : []), "tools.profiles"); unique(next.policies.flatMap(row => row.id ? [row.id] : []), "tools.policies"); for (const row of next.connections) requireItem(snapshot.connections, connection => connection.id === row.connectionId, `Connection ${row.connectionId}`); - for (const previous of before.connections) { + const connectionIds = new Set([...before.connections, ...next.connections].map(row => row.connectionId)); + for (const connectionId of connectionIds) { + const previous = before.connections.find(row => row.connectionId === connectionId) ?? { connectionId, installed: false }; const installed = next.connections.find(row => row.connectionId === previous.connectionId)?.installed ?? false; if (installed === previous.installed) continue; const connection = snapshot.connections.find(row => row.id === previous.connectionId)!; diff --git a/ui/src/pages/AgentAdvancedTab.test.tsx b/ui/src/pages/AgentAdvancedTab.test.tsx index f2a81500f97..1f1741b20e9 100644 --- a/ui/src/pages/AgentAdvancedTab.test.tsx +++ b/ui/src/pages/AgentAdvancedTab.test.tsx @@ -63,7 +63,7 @@ describe("Advanced agent configuration", () => { 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.replace("budgetMonthlyCents: 0", "budgetMonthlyCents: -1")); + 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")); @@ -72,7 +72,7 @@ describe("Advanced agent configuration", () => { 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()).toContain("budgetMonthlyCents: 0")); + await settle(() => expect(editor().state.doc.toString()).not.toContain("budgetMonthlyCents:")); expect(button("Save changes").disabled).toBe(true); }); @@ -84,7 +84,7 @@ describe("Advanced agent configuration", () => { 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.replace("title: null", "title: Engineer")); + 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)); @@ -97,7 +97,7 @@ describe("Advanced agent configuration", () => { throw new ConfigurationSaveInterrupted("second key", ["first key"], new Error("Network lost")); }); await render(); await settle(() => expect(editor()).toBeTruthy()); - await edit(source => source.replace("title: null", "title: Engineer")); + 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); @@ -106,8 +106,8 @@ describe("Advanced agent configuration", () => { 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()).toContain("title: null")); - await edit(source => source.replace("title: null", "title: Lead")); + await settle(() => expect(editor().state.doc.toString()).not.toContain("title:")); + await edit(source => `${source} title: Lead\n`); expect(button("Save changes").disabled).toBe(false); }); @@ -115,7 +115,7 @@ describe("Advanced agent configuration", () => { let complete!: (value: ReturnType) => void; api.save.mockImplementation(() => new Promise(resolve => { complete = resolve; })); await render(); await settle(() => expect(editor()).toBeTruthy()); - await edit(source => source.replace("title: null", "title: Engineer")); + 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(); @@ -130,7 +130,7 @@ describe("Advanced agent configuration", () => { 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.replace("apiKeys: []", "apiKeys: [changed]")); + 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(); diff --git a/ui/src/pages/AgentAdvancedTab.tsx b/ui/src/pages/AgentAdvancedTab.tsx index efd6bf629fc..b2eaa9baba5 100644 --- a/ui/src/pages/AgentAdvancedTab.tsx +++ b/ui/src/pages/AgentAdvancedTab.tsx @@ -13,6 +13,9 @@ import { queryKeys } from "../lib/queryKeys"; import { parseAgentConfigurationYaml, serializeAgentConfigurationDocument } from "../lib/agent-configuration-yaml"; import { loadAgentConfiguration, type AgentConfigurationSnapshot } from "../lib/agent-configuration/load"; import { ConfigurationSaveInterrupted, planAgentConfiguration, saveAgentConfiguration, type ConfigurationReceipt } from "../lib/agent-configuration/save"; +import { compactAgentConfiguration } from "../lib/agent-configuration/compact"; + +const editableYaml = (configuration: AgentConfigurationSnapshot) => serializeAgentConfigurationDocument(compactAgentConfiguration(configuration.document)); const yamlHighlightStyle = HighlightStyle.define([ { tag: tags.propertyName, class: "text-primary" }, @@ -53,7 +56,7 @@ function ConfigurationEditor({ agent, initial, onDirtyChange, onSaved, onSavingC const queryClient = useQueryClient(); const { pushToast } = useToastActions(); const [saved, setSaved] = useState(initial); - const [source, setSource] = useState(() => serializeAgentConfigurationDocument(initial.document)); + const [source, setSource] = useState(() => editableYaml(initial)); const [error, setError] = useState(null); const [reloadRequired, setReloadRequired] = useState(false); const [progress, setProgress] = useState(""); @@ -61,7 +64,7 @@ function ConfigurationEditor({ agent, initial, onDirtyChange, onSaved, onSavingC const operationPending = useRef(false); const active = useRef(true); useEffect(() => { active.current = true; return () => { active.current = false; }; }, []); - const original = useMemo(() => serializeAgentConfigurationDocument(saved.document), [saved]); + const original = useMemo(() => editableYaml(saved), [saved]); const dirty = source !== original; const validation = useMemo(() => { try { @@ -88,7 +91,7 @@ function ConfigurationEditor({ agent, initial, onDirtyChange, onSaved, onSavingC queryClient.setQueryData(["agent-configuration-yaml", agent.companyId, agent.id], configuration); if (!active.current) return; setSaved(configuration); - setSource(serializeAgentConfigurationDocument(configuration.document)); + setSource(editableYaml(configuration)); setError(null); onDirtyChange(false); setReloadRequired(false); @@ -113,7 +116,7 @@ function ConfigurationEditor({ agent, initial, onDirtyChange, onSaved, onSavingC queryClient.invalidateQueries({ queryKey: queryKeys.agents.detail(agent.id) }); queryClient.invalidateQueries({ queryKey: queryKeys.agents.detail(agent.urlKey) }); queryClient.invalidateQueries({ queryKey: queryKeys.agents.list(agent.companyId) }); - if (active.current) { setSaved(updated); setSource(serializeAgentConfigurationDocument(updated.document)); setError(null); setReloadRequired(false); onSaved(updated.agent); } + if (active.current) { setSaved(updated); setSource(editableYaml(updated)); setError(null); setReloadRequired(false); onSaved(updated.agent); } }, onSettled: () => { operationPending.current = false; }, onError: cause => setError(cause.message), @@ -122,6 +125,7 @@ function ConfigurationEditor({ agent, initial, onDirtyChange, onSaved, onSavingC useEffect(() => { onSavingChange?.(busy); }, [busy, onSavingChange]); useEffect(() => () => onSavingChange?.(false), [onSavingChange]); return
+

Omitted settings stay unchanged. Instruction file contents, inherited access, and history are available in their settings tabs.

{receipts.length > 0 &&
{receipts.map((receipt, index) =>

{receipt.label}