From 4808a3a2b1d3a447bdda5fac83f043fb2fabfb32 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 28 Oct 2026 11:46:58 -0500 Subject: [PATCH 1/2] fix(heartbeat): retry operator Stop after a transaction deadlock (#17895) https://entire.com/gh/paperclipai/-/trails/69/changes/paperclip/69 ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operator Stop must revoke task authority before it stops provider execution. > - Its transaction locks the task and then the run. A concurrent writer can hold the run while it waits for the task. > - PostgreSQL can abort the Stop transaction with a confirmed deadlock. The cancel API then returns 500. > - This pull request retries only that aborted database transaction, at most three times. > - Provider Stop and activity publication stay after commit, so the retry cannot repeat those effects. ## Linked Issues or Issue Description Refs #17639. Its agent-chat project browser test completed the product assertions, then failed during cleanup when the cancel API returned 500. A real PostgreSQL lock-cycle test reproduces the same `40P01` error in the Stop transaction. Related: #13911 addresses audit foreign-key lock contention. This change handles a different concurrent run-writer path without changing the schema or task/run lock order. Stack base: #17639. This is a separate correctness repair for the sleep migration stack, not another polling migration. ## What Changed - Control the runner timeout regression fixture's clock so it reaches the pending append before expiry; retain production cancellation and late-append assertions. - Retry the Stop authority transaction only after PostgreSQL reports `40P01`. Make at most three attempts, with short bounded backoff. - Discard rolled-back local run state, claim flags, and queued activity publications before each retry. Re-read current authority under fresh locks. - Keep provider cancellation and activity publication outside the retried transaction. Preserve other errors and the existing native lock-conflict response. - Add a real two-transaction deadlock test. Add server-side failure tests for retry exhaustion and errors that must not retry. - Let run-control tests use a validated disposable PostgreSQL database URL. ## Verification Published head: `60ba892ae9fc5faaf8cd0833694b4a66c13323c3`, targeting master after #17639 merged at `1ed0af41867ba40fd1d23292d395588d18c2fcbe`. The normal ancestry merge preserves the complete five-path tested cancellation patch byte-for-byte, including the review repair, and all four unrelated current master paths. The production retry logic remains unchanged from this slice's reviewed implementation; the follow-up adds coverage and its audit. - **228 distinct complete cases pass:** all 48 run-control cases and all 180 native-session runtime cases. The two new targeted cases are included in 48 and are not counted again. Server typechecking, build and module-boundary checks also pass. - The new real PostgreSQL cases reach a failed native run's correlated Stop claim, cancellation fence, board handoff retirement and activity write before raising `40P01`. One case preserves retry authority and commits exactly one cancellation activity/live notice. The other changes coordinator authority between attempts, returns 409, preserves the failed run and handoff, and performs no provider/workspace cleanup. - A negative control removes only the publication-list reset. It fails with two live notices for one durable activity. Production was restored byte-for-byte before final validation. The existing actual two-connection deadlock, bounded attempts, non-deadlock rejection and post-commit provider behavior remain covered. - Preserve the initial new-assertion typecheck failure (optional live-event payload) and the corrected final pass. Service-local test files are typechecked; `src/__tests__` is excluded. The negative-control harness initially expected an untruncated Vitest assertion line; the actual named 1-versus-2 failure and restoration were inspected before correcting its result matcher. - [Current-parent acceptance and failure ledger](https://github.com/paperclipai/paperclip/blob/60ba892ae9fc5faaf8cd0833694b4a66c13323c3/doc/plans/2026-10-28-cancellation-deadlock-acceptance.md) records current scope, commands, isolated fixtures, mutation evidence and limits. Earlier combined-stack counts retain their original revision scope. The existing controlled runner timeout/late-append fixture is retained and passes within the complete 180-case runtime suite. - This addresses the prior Ponytail request to exercise native claimed state before deadlock rollback and re-evaluate authority afterward. The advisory loop-invariant snapshot/named-attempt-bound note remains a maintainability suggestion, not a runtime defect. No production loop refactor or new worker duty is introduced. - **All 58 current checks are clean; Greptile and Ponytail both rate this exact head 5/5.** Ponytail explicitly confirms that the native-claim coverage request is resolved and says Ship. The formerly queued dependency/quality review workflow has completed successfully. Old `fffc`/parent checks and reviews are not current passes; the former pending Snyk status is not called successful. Actual failing Ponytail findings must be fixed. Missing/stopped/paused turns without a verdict are authorized nonblockers. Active unfinished reviews retain their original per-head timer, with a 25-minute monitor cutoff respecting the 30-minute maximum. - No full local repository matrix, live provider execution, external alarm, physical host/database suspension or savings claim is made. No schema, dependency, workflow, shared scheduler contract or shared OS resource changes are introduced. ## Risks - Persistent contention still returns an error after the third attempt. This change does not promise to eliminate every possible lock cycle. - Retrying a write after an ambiguous connection failure would be unsafe. The retry accepts only PostgreSQL's confirmed deadlock abort code; it does not accept transport errors. - No schema or public API contract changes. Current task/run authority and cancellation eligibility are checked again on each attempt. ## Model Used OpenAI GPT-6 via Codex, with shell, GitHub CLI, TypeScript, PostgreSQL, and Playwright tools. The exact serving model ID and context window are not exposed in this session. ## 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 --- ...-10-28-cancellation-deadlock-acceptance.md | 77 ++++++++++ .../src/native-session-runtime.test.ts | 60 ++++---- .../__tests__/run-control-fixtures.ts | 11 +- .../services/heartbeat/run-control.test.ts | 133 +++++++++++++++++- server/src/services/heartbeat/run-control.ts | 24 +++- 5 files changed, 271 insertions(+), 34 deletions(-) create mode 100644 doc/plans/2026-10-28-cancellation-deadlock-acceptance.md diff --git a/doc/plans/2026-10-28-cancellation-deadlock-acceptance.md b/doc/plans/2026-10-28-cancellation-deadlock-acceptance.md new file mode 100644 index 00000000000..491240ace26 --- /dev/null +++ b/doc/plans/2026-10-28-cancellation-deadlock-acceptance.md @@ -0,0 +1,77 @@ +# Cancellation deadlock retry acceptance + +This existing correctness slice (#17895) retries only the operator Stop database +transaction after PostgreSQL reports `40P01`, at most three times. Provider Stop, +workspace settlement, and activity publication remain after commit. This is not +a new polling or scheduling duty. + +## Current parent and outstanding review + +The previous published head is +`fffc8d0c46fbbeb3da4a23a36e638fefd3b8fe3d`, originally based on native progress +`08d07ead105dc89c2af9b62f4d90bff8cc93540c`. The slice is prepared by a normal +merge of verified native progress `6f365285f5b233a89acc714eadfead5ab145100e`. +All four original changed-file hunks retain their exact content and context. +Only the run-control blob index hashes differ because the parent also changed +that file. The automatic composition is clean and preserves those parent edits. + +The prior Ponytail review said Ship with an actionable coverage request: existing +deadlock tests did not reach a correlated native failed run's claimed state before +rollback. The follow-up adds two real PostgreSQL cases in the existing run-control +suite, with no new production behavior: + +1. Keep retry authority: claim the correlated Stop, fence the failed native run, + cancel its real board terminal handoff, and persist cancellation activity inside + the first transaction. Raise PostgreSQL `40P01` before commit. Retry succeeds; + exactly one cancellation activity is durable and exactly one live notice is + published. The correlation belongs to the committed attempt. +2. Revoke retry authority: after that aborted transaction rolls back, an independent + coordinator update retires retry eligibility. The second attempt returns 409, + preserves the failed run and uncancelled handoff, publishes no cancellation + activity, and performs no provider or workspace cleanup. + +The existing actual two-connection lock-cycle regression and the bounded retry/ +non-deadlock cases remain in the complete suite. The new tests exercise claims +and activity produced before a real PostgreSQL transaction abort; they do not +infer correctness from a mocked retry callback alone. + +## Evidence and failure accounting + +- Both new cases pass against the retained production reset logic. +- Removing only `publications.length = 0` makes the preserved-authority case fail: + two live cancellation notices appear for one committed activity. Production was + restored byte-for-byte to its integrated index before further validation. + `/tmp/sleep-aux-native-retry-mutation-proof.json` records that comparison. +- The negative-control harness initially expected an untruncated assertion line; + Vitest abbreviates that line. Its actual named failure and 1-versus-2 result were + inspected, and the matcher was corrected without rerunning or altering production. +- Typechecking caught an optional live-event payload in the new assertion. The + assertion now uses optional access. Preserve the original failed typecheck at + `/tmp/sleep-aux-current-parent-types.log`; the final typecheck passes at + `/tmp/sleep-aux-current-parent-types-final.log`. +- **228 distinct complete cases pass:** all 48 run-control cases, including the two + additions, and all 180 native-session runtime cases, including the existing + controlled timeout/late-append fixture. The two targeted probes are included in + 48 and are not counted again. +- Server typechecking, build, and module-boundary checks pass. `server/tsconfig.json` + excludes `src/__tests__`, but includes service-local test files such as + `run-control.test.ts`; typechecking does not replace test execution. Earlier + statements about tests omitted by production typechecking must be read with + that path boundary. + +The three PostgreSQL probes use separate new disposable databases on the owned +local cluster. Full-run results and exit receipts are in +`/tmp/sleep-aux-current-parent-{types-final,build,boundaries,control,runner}.log`. +The targeted positive and mutation logs are +`/tmp/sleep-aux-native-retry-regression.log` and +`/tmp/sleep-aux-native-retry-mutant-red.log`. + +## Remaining gates and limits + +This is local preparation. The dependency must merge, the normal merge onto its +actual landed master must preserve this tested patch and all master changes, and +fresh exact-head CI plus Greptile 5/5 must pass before protected merge. Prior +review scores and checks on older heads are not current passes. The previous +Snyk status was pending and is not called successful here. No shared OS resource +cleanup, schema change, whole-repository local matrix, live provider execution, +external alarm, physical suspension, or savings qualification is claimed. diff --git a/packages/paperclip-runner/src/native-session-runtime.test.ts b/packages/paperclip-runner/src/native-session-runtime.test.ts index 588b64556df..caba0b66d12 100644 --- a/packages/paperclip-runner/src/native-session-runtime.test.ts +++ b/packages/paperclip-runner/src/native-session-runtime.test.ts @@ -3499,10 +3499,6 @@ describe("executeNativeSession recovery", () => { }); it("stops and closes a timed-out consumer even when the caller requested a warm session", async () => { - let markAppendStarted = () => {}; - const appendStarted = new Promise((resolve) => { - markAppendStarted = resolve; - }); let releaseAppend = () => {}; const appendReleased = new Promise((resolve) => { releaseAppend = resolve; @@ -3515,7 +3511,6 @@ describe("executeNativeSession recovery", () => { let appendCommitted = false; const appendEvent = vi.fn( async (_event: PrpEvent, options?: { signal: AbortSignal }) => { - markAppendStarted(); await Promise.race([ appendReleased, new Promise((_resolve, reject) => { @@ -3612,28 +3607,39 @@ describe("executeNativeSession recovery", () => { async completeRun() {}, }; - const execution = executeNativeSession({ - input, - backend, - controlPlane: port, - runnerInstanceId: "runner-recovery", - controlPlaneInstanceId: "control-recovery", - timeoutMs: 1, - keepSessionOpen: true, - }); - const rejection = expect(execution).rejects.toThrow( - "native session timed out", - ); - await appendStarted; - await vi.waitFor(() => expect(iteratorTeardown).toHaveBeenCalledOnce()); - await rejection; - expect(cancel).toHaveBeenCalledOnce(); - expect(close).toHaveBeenCalled(); - expect(appendEvent).toHaveBeenCalledOnce(); - expect(appendCommitted).toBe(false); - releaseAppend(); - await new Promise((resolve) => setImmediate(resolve)); - expect(appendCommitted).toBe(false); + // Reach the pending append before expiring the consumer. A real 1 ms + // deadline can expire during setup before the consumer reaches this case. + vi.useFakeTimers(); + try { + const execution = executeNativeSession({ + input, + backend, + controlPlane: port, + runnerInstanceId: "runner-recovery", + controlPlaneInstanceId: "control-recovery", + timeoutMs: 1, + keepSessionOpen: true, + }); + const rejection = expect(execution).rejects.toThrow( + "native session timed out", + ); + await vi.advanceTimersByTimeAsync(0); + expect(appendEvent).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(1); + await rejection; + expect(iteratorTeardown).toHaveBeenCalledOnce(); + expect(cancel).toHaveBeenCalledOnce(); + expect(close).toHaveBeenCalled(); + expect(appendEvent).toHaveBeenCalledOnce(); + expect(appendCommitted).toBe(false); + releaseAppend(); + await vi.advanceTimersByTimeAsync(0); + expect(appendCommitted).toBe(false); + } finally { + releaseAppend(); + releaseTeardown(); + vi.useRealTimers(); + } }); it("closes a failed session while retaining an uncancellable event read", async () => { diff --git a/server/src/services/heartbeat/__tests__/run-control-fixtures.ts b/server/src/services/heartbeat/__tests__/run-control-fixtures.ts index 2adce9077c1..06ed71c35d1 100644 --- a/server/src/services/heartbeat/__tests__/run-control-fixtures.ts +++ b/server/src/services/heartbeat/__tests__/run-control-fixtures.ts @@ -1,8 +1,9 @@ import type { Db } from "@paperclipai/db"; -import { agents, companies, createDb, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS, heartbeatRuns } from "@paperclipai/db"; +import { agents, companies, createDb, applyPendingMigrations, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS, heartbeatRuns } from "@paperclipai/db"; import { sql } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, beforeEach, vi, type Mock } from "vitest"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../../../__tests__/helpers/embedded-postgres.js"; +import { resolveExternalTestDatabaseUrl } from "../../../__tests__/helpers/external-test-database.js"; import { runningProcesses } from "../../../adapters/index.js"; import { terminateLocalService } from "../../local-service-supervisor.js"; import { createHeartbeatRunState } from "../run-state.js"; @@ -65,15 +66,17 @@ export function callbacks(db: Db): TestCallbacks { failedProcessRunCancellations: new Map; failed: boolean; error?: unknown }>(), } satisfies HeartbeatRunControlDependencies; } -export const runControlPostgresSupport = await getEmbeddedPostgresTestSupport(); +const externalTestDatabaseUrl = resolveExternalTestDatabaseUrl("PAPERCLIP_RUN_CONTROL_TEST_DATABASE_URL"); +export const runControlPostgresSupport = externalTestDatabaseUrl ? { supported: true, reason: undefined } : await getEmbeddedPostgresTestSupport(); if (!runControlPostgresSupport.supported) console.warn(`Skipping run-control database tests: ${runControlPostgresSupport.reason}`); export function setupRunControlDatabase(name: string, ready: (db: Db) => void) { let database: Awaited>; let db: Db; beforeAll(async () => { - database = await startEmbeddedPostgresTestDatabase(name); - db = createDb(database.connectionString); + if (externalTestDatabaseUrl) await applyPendingMigrations(externalTestDatabaseUrl); + else database = await startEmbeddedPostgresTestDatabase(name); + db = createDb(externalTestDatabaseUrl ?? database!.connectionString); ready(db); }, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS); afterAll(async () => { diff --git a/server/src/services/heartbeat/run-control.test.ts b/server/src/services/heartbeat/run-control.test.ts index ab0a9fbb5a8..c73dba4d120 100644 --- a/server/src/services/heartbeat/run-control.test.ts +++ b/server/src/services/heartbeat/run-control.test.ts @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import type { Db } from "@paperclipai/db"; import { activityLog, agents, agentWakeupRequests, caseEvents, caseHandoffs, cases, companies, environmentLeases, heartbeatRuns, issueComments, issues, nativeRunFinalizations } from "@paperclipai/db"; -import { eq, inArray } from "drizzle-orm"; +import { eq, inArray, sql } from "drizzle-orm"; import { afterEach, describe, expect, it, vi } from "vitest"; import { runningProcesses } from "../../adapters/index.js"; import { @@ -14,6 +14,7 @@ import { AdapterStopTimeoutError } from "../adapter-stop-timeout.js"; import { HttpError } from "../../errors.js"; import { captureException } from "../../sentry.js"; import { terminateLocalService } from "../local-service-supervisor.js"; +import { publishLiveEvent } from "../live-events.js"; import { createHeartbeatRunState } from "./run-state.js"; import { createHeartbeatRunControl } from "./run-control.js"; import { preserveCancelledNativeWorkspaces } from "../native-runtime/native-failed-workspace-recovery.js"; @@ -173,6 +174,79 @@ describe.skipIf(!runControlPostgresSupport.supported)("heartbeat run-control dat )).toMatchObject({ status: "cancelled" }); }); + it.each([["40P01", 3], ["XX000", 1]] as const)("bounds cancellation transaction attempts for PostgreSQL %s", async (code, attempts) => { + const { company, agent } = await fixture(); + const run = await liveRun(company.id, agent.id); + const [issue] = await db.insert(issues).values({ companyId: company.id, title: "Cancellation failure", status: "in_progress", + assigneeAgentId: agent.id, executionRunId: run.id }).returning(); + await db.update(heartbeatRuns).set({ issueId: issue.id, contextSnapshot: { issueId: issue.id } }).where(eq(heartbeatRuns.id, run.id)); + await db.execute(sql`create sequence run_control_cancel_attempts`); + await db.execute(sql.raw(`create function run_control_cancel_fault() returns trigger language plpgsql as $$ + begin + if new.result_json->'executionCancellation'->>'state' = 'requested' then + perform nextval('run_control_cancel_attempts'); + raise exception using errcode = '${code}', message = 'Synthetic cancellation transaction failure'; + end if; + return new; + end $$`)); + await db.execute(sql`create trigger run_control_cancel_fault before update on heartbeat_runs + for each row execute function run_control_cancel_fault()`); + const deps = callbacks(db); + try { + await expect(createHeartbeatRunControl(db, deps).cancelRunInternal(run.id, "Operator Stop", { + resultJson: { cancelledByActorType: "user", cancelledByUserId: "operator" }, + })).rejects.toMatchObject({ cause: { code } }); + const [counter] = await db.execute<{ count: number }>(sql`select last_value::int as count from run_control_cancel_attempts`); + expect(counter.count).toBe(attempts); + expect(await deps.getRun(run.id)).toMatchObject({ status: "running", resultJson: null }); + expect(terminateLocalService).not.toHaveBeenCalled(); + expect(deps.appendRunEvent).not.toHaveBeenCalled(); + } finally { + await db.execute(sql`drop trigger run_control_cancel_fault on heartbeat_runs`); + await db.execute(sql`drop function run_control_cancel_fault()`); + await db.execute(sql`drop sequence run_control_cancel_attempts`); + } + }); + + it("finishes operator Stop after a concurrent run writer causes a transaction deadlock", async () => { + const { company, agent } = await fixture(); + const run = await liveRun(company.id, agent.id); + const [issue] = await db.insert(issues).values({ companyId: company.id, title: "Stop contention", status: "in_progress", + assigneeAgentId: agent.id, executionRunId: run.id, checkoutRunId: run.id }).returning(); + await db.update(heartbeatRuns).set({ issueId: issue.id, contextSnapshot: { issueId: issue.id } }).where(eq(heartbeatRuns.id, run.id)); + let enter!: () => void, release!: () => void; + const entered = new Promise(resolve => { enter = resolve; }); + const released = new Promise(resolve => { release = resolve; }); + let blockerPid = 0; + const writer = db.transaction(async tx => { + // Make Stop's default detector see the cycle first, so this reproduces + // the API-side victim observed in CI rather than a random victim. + await tx.execute(sql`set local deadlock_timeout = '10s'`); + const [backend] = await tx.execute<{ pid: number }>(sql`select pg_backend_pid() as pid`); + blockerPid = backend.pid; + await tx.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run.id)).for("update"); + enter(); await released; + await tx.update(issues).set({ updatedAt: new Date() }).where(eq(issues.id, issue.id)); + }); + await entered; + const deps = callbacks(db); + const stopping = createHeartbeatRunControl(db, deps).cancelRunInternal(run.id, "Operator Stop", { + resultJson: { cancelledByActorType: "user", cancelledByUserId: "operator" }, + }).then(value => ({ value, error: undefined }), error => ({ value: undefined, error })); + try { + await vi.waitFor(async () => { + const waiting = await db.execute(sql`select pid from pg_stat_activity + where ${blockerPid} = any(pg_blocking_pids(pid)) and wait_event_type = 'Lock'`); + expect(waiting.length).toBeGreaterThan(0); + }, { timeout: 5000 }); + } finally { release(); } + await writer; + const result = await stopping; + expect(result.error).toBeUndefined(); + expect(result.value).toMatchObject({ status: "cancelled" }); + expect(await deps.getRun(run.id)).toMatchObject({ status: "cancelled" }); + }); + it.each(["queued", "running"] as const)("preserves case intent when a guarded Stop loses to %s promotion", async status => { const { company, agent } = await fixture(); const [issue] = await db.insert(issues).values({ companyId: company.id, title: "Promoted case work", status: "todo", assigneeAgentId: agent.id }).returning(); @@ -393,6 +467,63 @@ describe.skipIf(!runControlPostgresSupport.supported)("heartbeat run-control dat .toMatchObject({ status: "in_progress", phase: "verification", completedAt: null }); }); + it.each([false, true])("discards rolled-back native Stop claims and activity before retry (authority revoked=%s)", async revokeAuthority => { + const f = await boardTerminalFixture("running", "native"); + const requestId = randomUUID(), deps = callbacks(db); + vi.mocked(publishLiveEvent).mockClear(); + const transact = db.transaction.bind(db); + let injected = false; + const transaction = vi.spyOn(db, "transaction").mockImplementationOnce(async (operation, config) => { + try { + return await transact(async tx => { + const result = await operation(tx); + const [claimed] = await tx.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.run.id)); + expect(claimed.resultJson?.startupCancellation).toMatchObject({ cancellationRequestId: requestId }); + expect(claimed.resultJson?.executionCancellation).toMatchObject({ state: "requested" }); + expect((await tx.select().from(caseHandoffs).where(eq(caseHandoffs.id, f.receipt.handoffId)))[0].state).toBe("cancelled"); + expect((await tx.select().from(activityLog).where(eq(activityLog.companyId, f.company.id))) + .filter(row => row.action === "case.handoff_cancelled")).toHaveLength(1); + expect(deps.getAgent).not.toHaveBeenCalled(); + injected = true; + await tx.execute(sql`do $$ begin raise exception using errcode = '40P01', message = 'Deadlock after native Stop claim'; end $$`); + return result; + }, config); + } catch (error) { + if (injected && revokeAuthority) await db.update(nativeRunFinalizations) + .set({ phase: "terminal_failure", failureCode: "retry_authority_revoked", nextAttemptAt: null }) + .where(eq(nativeRunFinalizations.runId, f.run.id)); + throw error; + } + }); + try { + const stopping = createHeartbeatRunControl(db, deps).cancelRunInternal(f.run.id, "Retry native Stop", { + resultJson: { cancelledByActorType: "user", cancelledByUserId: "operator" }, + cancellationRequestId: requestId, cancellationRequestedByUserId: "operator", + }); + if (revokeAuthority) { + await expect(stopping).rejects.toMatchObject({ status: 409 }); + expect(await deps.getRun(f.run.id)).toMatchObject({ status: "failed", resultJson: null }); + expect(deps.getAgent).not.toHaveBeenCalled(); + expect(deps.options.closeWarmNativeSessionsForRun).not.toHaveBeenCalled(); + expect(deps.envOrchestrator.releaseForRun).not.toHaveBeenCalled(); + } else { + await expect(stopping).resolves.toMatchObject({ status: "cancelled" }); + expect((await deps.getRun(f.run.id))?.resultJson?.startupCancellation).toMatchObject({ cancellationRequestId: requestId }); + } + expect(injected).toBe(true); + expect(transaction.mock.calls.length).toBeGreaterThanOrEqual(2); + expect((await db.select().from(caseHandoffs).where(eq(caseHandoffs.id, f.receipt.handoffId)))[0].state) + .toBe(revokeAuthority ? "awaiting_completion" : "cancelled"); + const expected = revokeAuthority ? 0 : 1; + expect((await db.select().from(activityLog).where(eq(activityLog.companyId, f.company.id))) + .filter(row => row.action === "case.handoff_cancelled")).toHaveLength(expected); + expect(vi.mocked(publishLiveEvent).mock.calls.map(([event]) => event) + .filter(event => event.companyId === f.company.id && event.type === "activity.logged" + && event.payload?.action === "case.handoff_cancelled")).toHaveLength(expected); + expect(terminateLocalService).not.toHaveBeenCalled(); + } finally { transaction.mockRestore(); } + }); + it("stops current preparing execution despite a prior terminal checkout when cancelling board intent", async () => { // Queue admission binds the new execution before preparation adopts its // predecessor's checkout. The execution owner is authoritative in this gap. diff --git a/server/src/services/heartbeat/run-control.ts b/server/src/services/heartbeat/run-control.ts index c1b50a46517..c32cebc2013 100644 --- a/server/src/services/heartbeat/run-control.ts +++ b/server/src/services/heartbeat/run-control.ts @@ -1134,7 +1134,7 @@ export function createHeartbeatRunControl(db: Db, dependencies: HeartbeatRunCont let caseHandoffRetryClaimed = false; if (handoffIssueId && supersedesCaseHandoff && options.resultJson?.caseHandoffYieldStop !== true) { const publications: ActivityPublication[] = []; - const supersessionApplied = await db.transaction(async tx => { + const applySupersession = () => db.transaction(async tx => { const [issue] = await tx.select({ id: issues.id, executionRunId: issues.executionRunId, checkoutRunId: issues.checkoutRunId, assigneeAgentId: issues.assigneeAgentId, assigneeUserId: issues.assigneeUserId }).from(issues) .where(and(eq(issues.companyId, run!.companyId), eq(issues.id, handoffIssueId))).for("update"); @@ -1189,7 +1189,27 @@ export function createHeartbeatRunControl(db: Db, dependencies: HeartbeatRunCont await cancelCaseHandoffsForIssue(tx as unknown as Db, { companyId: run!.companyId, issueId: handoffIssueId, sourceRunId: runId, includeUnboundTerminalIntent: Boolean(fenced && ownsTask), reason, publications }); return true; - }).catch(rethrowNativeCancellationLockConflict); + }); + let supersessionApplied = false; + for (let attempt = 0; ; attempt++) { + const previousRun = run; + try { + supersessionApplied = await applySupersession(); + break; + } catch (error) { + const value = parseObject(error); + const deadlock = value.code === "40P01" || parseObject(value.cause).code === "40P01"; + if (!deadlock || attempt >= 2) rethrowNativeCancellationLockConflict(error); + // PostgreSQL aborted the entire transaction. Discard its local + // receipts before re-reading current authority under fresh locks. + // Provider Stop and publication happen only after this boundary. + run = previousRun; + cancellationClaimed = false; + caseHandoffRetryClaimed = false; + publications.length = 0; + await new Promise(resolve => setTimeout(resolve, 25 * (attempt + 1))); + } + } publications.forEach(publishActivity); if (!supersessionApplied) return null; } From b14e1ecaf85ce6eafa05125f1b539e3ac7f32022 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 28 Oct 2026 11:49:24 -0500 Subject: [PATCH 2/2] feat: add a complete agent YAML configuration editor (#18036) https://entire.com/gh/paperclipai/-/trails/175/changes/paperclip/175 ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operators configure each agent through several settings tabs. > - Operators also need one complete, editable configuration document. > - The document must retain each resource service's access and validation rules. > - This pull request defines the `paperclip/v1` Agent YAML format and adds an Advanced editor. > - The editor previews changes, rejects stale core writes, and explains partial saves. > - Operators can edit agent settings in one place and verify the saved state after reload. ## Linked Issues or Issue Description Refs #2219. Related settings work: #10974. **Problem or motivation** Agent configuration spans profile, runtime, instructions, skills, tools, channels, permissions, secrets, keys, and revision history. Operators cannot inspect and edit those settings as one versioned YAML document. **Proposed solution** Add an Advanced agent tab with a YAML editor. Define a shared document schema. Read and write resources through their existing APIs. Separate editable settings, explicit commands, and read-only observed state. **Alternatives considered** A raw agent-row editor would omit related resources. A generic database update would bypass resource ownership. The editor therefore uses the existing resource contracts and records partial results when a later request fails. **Roadmap alignment** This extends the shipped agent configuration capability. It relates to easy AGENTS.md configuration, multi-harness teams, governed tool access, and secrets management. Stored credentials remain redacted. This format is an editable settings view; it does not add company portability or provider consent. ## What Changed - Add the `paperclip/v1` Agent document schema and format documentation. - Add the Advanced route, sidebar entry, YAML editor, validation, change summary, save progress, and reload recovery. - Cover core settings, instruction files, skill pins, tool installs/profiles/policies, channels, permissions, secret operations, API keys, and revision restore. - Add private-keyed configuration revisions checked under the agent write lock. Read the current key through its owner so file-based rotation does not require restarting the server. Preserve unchanged secret markers at their original paths. - Retain drafts and returned credentials after partial saves. Require reload before retrying commands. Prevent duplicate submissions. Keep new key tokens visible through rename and disable saves with an empty change plan. - Send only changed channel settings. Require explicit intent for shared profile edits. Load destination instruction files when changing their location. - Update Storybook fixtures, verification maps, and regression tests. Add browser acceptance through real APIs and PostgreSQL. ## Verification - `pnpm -r typecheck` passes. - `pnpm build` passes. - The focused UI suite passes all 50 tests. The API/revision helper suites pass all 93 tests. - All 65 lifecycle tests pass with a dedicated PostgreSQL database. They exercise real row locks, redaction, revision history, audit, and company access. - All six browser acceptance cases pass. They check saved settings and clears, secret redaction, key creation/revocation, stale drafts, lost-response recovery, instruction relocation, revision restore, and key-token retention through rename. The tests use real resource APIs and storage. The MCP target uses the existing local provider fixture. No model run or provider login is claimed. - Signing-key regressions cover both environment changes and real in-place key-file rotation. The protected database save/fencing cases pass after removing the cache. A local measurement of 1,000 file-backed revision calculations took 48 ms. - `pnpm check:token-gates`, `node scripts/check-maps.mjs`, and Storybook catalog checks pass. - The full [PR CI run](https://github.com/paperclipai/paperclip/actions/runs/40725425470) passes on `34ed97bbf9`: all 48 jobs, including all eight browser shards. Greptile is 5/5 on this commit, with no unresolved review threads. This final commit refreshes CI after a runner-routing fallback; its source tree matches `fb4cf488cf`. - Paperclip Review and Superagent Security Scan ended neutral because their external agents did not complete a valid assessment/receipt. Superagent passed on `fb4cf488cf`, which has the identical source tree. One request to retry the final scan returned HTTP 404. Neither check reports a code finding. - The local full-suite attempt was blocked by host System V semaphore exhaustion. Acceptance and lifecycle verification used an isolated PostgreSQL 18 container. Earlier CI runners were interrupted. A local combined run hit hook/test timeouts; the full 88-test route file passed after the builds finished. The final full CI run passes. Earlier failure logs and browser traces remain retained outside the repository. To run the browser regression: ```sh pnpm exec playwright test --config tests/e2e/playwright.config.ts tests/e2e/agent-configuration.spec.ts ``` ## Risks - Multi-resource saves are sequential. Earlier operations can remain saved if a later operation fails. The editor reports the completed steps and requires reload. - Core settings and managed instruction files have atomic revision/hash checks. Other unversioned resource writes and external supporting files retain a race window after preflight. - Shared profile definitions affect other bindings. The editor requires `updateSharedDefinition: true` for those edits. - The editor follows native service normalization and experimental settings. Canonical reload can add generated access profiles or mask environment values. - New editor dependencies require the repository's normal automated lockfile update. This pull request does not edit the lockfile or add a database migration. ## Model Used OpenAI Codex, GPT-6 family. The exact current model version was not exposed. The work used reasoning, repository tools, code execution, and browser automation. Earlier work in this chat used the configured Codex model before a model switch; its exact model ID and context window were not exposed. The current context window size is also not exposed. ## 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/PRODUCT.md | 6 + doc/SPEC-implementation.md | 9 + doc/agent-configuration-yaml.md | 347 ++++++++++++++++++ maps/agent-access-map/agents.md | 17 + maps/feature-map/agents.md | 15 + .../src/agent-configuration-resources.ts | 59 +++ packages/shared/src/agent-configuration.ts | 62 ++++ packages/shared/src/index.ts | 1 + packages/shared/src/validators/agent.ts | 1 + .../agent-adapter-validation-routes.test.ts | 47 +++ .../agent-configuration-revision.test.ts | 67 ++++ .../src/lib/agent-configuration-revision.ts | 32 ++ server/src/lib/agent-records.ts | 1 + .../__tests__/lifecycle.test.ts | 65 +++- .../agent-lifecycle/adapters/configuration.ts | 4 + server/src/routes/agents.ts | 22 +- tests/e2e/agent-configuration.spec.ts | 241 ++++++++++++ ui/package.json | 7 + ui/src/api/agents.ts | 4 +- .../AgentContextualSidebar.test.tsx | 1 + ui/src/components/AgentContextualSidebar.tsx | 2 + ui/src/index.css | 20 + ui/src/lib/agent-configuration-yaml.test.ts | 54 +++ ui/src/lib/agent-configuration-yaml.ts | 68 ++++ ui/src/lib/agent-configuration/channels.ts | 52 +++ ui/src/lib/agent-configuration/load.ts | 119 ++++++ ui/src/lib/agent-configuration/save.test.ts | 315 ++++++++++++++++ ui/src/lib/agent-configuration/save.ts | 208 +++++++++++ ui/src/lib/agent-configuration/steps.ts | 18 + ui/src/lib/agent-configuration/tools.ts | 70 ++++ ui/src/pages/AgentAdvancedTab.test.tsx | 153 ++++++++ ui/src/pages/AgentAdvancedTab.tsx | 184 ++++++++++ ui/src/pages/AgentDetail.tsx | 26 +- ui/src/pages/agent-detail-navigation.test.ts | 3 + ui/src/pages/agent-detail-navigation.ts | 3 + .../agent-settings/AgentSettingsPreview.tsx | 7 +- .../prototypes/agent-settings/fixtures.ts | 20 +- .../agent-settings-redesign.stories.tsx | 8 + 38 files changed, 2325 insertions(+), 13 deletions(-) create mode 100644 doc/agent-configuration-yaml.md create mode 100644 packages/shared/src/agent-configuration-resources.ts create mode 100644 packages/shared/src/agent-configuration.ts create mode 100644 server/src/__tests__/agent-configuration-revision.test.ts create mode 100644 server/src/lib/agent-configuration-revision.ts create mode 100644 tests/e2e/agent-configuration.spec.ts create mode 100644 ui/src/lib/agent-configuration-yaml.test.ts create mode 100644 ui/src/lib/agent-configuration-yaml.ts create mode 100644 ui/src/lib/agent-configuration/channels.ts create mode 100644 ui/src/lib/agent-configuration/load.ts create mode 100644 ui/src/lib/agent-configuration/save.test.ts create mode 100644 ui/src/lib/agent-configuration/save.ts create mode 100644 ui/src/lib/agent-configuration/steps.ts create mode 100644 ui/src/lib/agent-configuration/tools.ts create mode 100644 ui/src/pages/AgentAdvancedTab.test.tsx create mode 100644 ui/src/pages/AgentAdvancedTab.tsx diff --git a/doc/PRODUCT.md b/doc/PRODUCT.md index 2acf62e5248..c4c28fedf37 100644 --- a/doc/PRODUCT.md +++ b/doc/PRODUCT.md @@ -49,6 +49,12 @@ start execution. This preference does not change the org chart or permissions. ### Agent Execution +Agents have an **Advanced** tab for editing their settings across all configuration tabs +as a versioned YAML document. Instructions, skills, tools, channels, trust, +secret operations, API keys, and revision restore use their existing domain APIs. +It edits the same configuration as the regular settings pages, validates before saving, preserves masked secrets, and rejects +stale edits. See [Agent configuration YAML](agent-configuration-yaml.md). + Existing Claude, Codex, and Grok Build agents can upgrade from the legacy runner on their Harness / Runtime page when Paperclip Runner is the qualified default for their environment. One action saves pending edits and translates compatible settings diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 01cae913f32..bac9acad7b9 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -324,6 +324,15 @@ Invariants: - no cycles in reporting tree - `terminated` agents cannot be resumed +Agent configuration also has a versioned YAML representation in the Advanced +tab, including instruction text, skills, tools, channels, permissions, secrets, +API key operations, and revision restore. Resource saves use their owning APIs +and expose partial failures; they are not a cross-resource transaction. +[Agent configuration YAML](agent-configuration-yaml.md) defines its schema, +replacement semantics, secret preservation, and revision preconditions. It uses +the existing configuration and resource APIs, retaining their authorization +and approval gates. Agent lifecycle commands remain separate actions. + ## 7.3 `agent_api_keys` - `id` uuid pk diff --git a/doc/agent-configuration-yaml.md b/doc/agent-configuration-yaml.md new file mode 100644 index 00000000000..c985b734750 --- /dev/null +++ b/doc/agent-configuration-yaml.md @@ -0,0 +1,347 @@ +# Agent configuration YAML + +Status: v1, October 28, 2026. + +The **Advanced** tab at `/:companyPrefix/agents/:agentRef/advanced` edits an +agent's settings as one YAML document. It covers the configuration tabs through +their existing APIs. The database and resource services remain authoritative; +this is not a file on the agent's computer or a harness-specific format. + +## Coverage + +| Settings surface | YAML location | Save behavior | +| --- | --- | --- | +| 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`. | +| 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`. | +| 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`. | + +Read-only information stays read-only: organization-wide inherited access, +immutable history, binary instruction assets, live status/spend, and provider +connection state. Interactive provider login/OAuth and approval flows still use +their normal UI. The YAML can select existing accounts/connections and configure +channel credentials; it cannot synthesize provider consent or export stored +credential values. AgentMail provisioning remains owned by its connection flow. + +## Document contract + +```yaml +apiVersion: paperclip/v1 +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 + cwd: /workspace/project + env: + SERVICE_TOKEN: + type: secret_ref + secretId: 11111111-1111-4111-8111-111111111111 + version: latest + runtimeConfig: + 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 + tools: + connections: [] + profiles: [] + policies: [] + channels: [] + secrets: + create: [] + rotate: [] + update: [] + proposals: [] + channelOperations: + create: [] + setup: [] + revokeIdentityLinks: [] + apiKeys: + create: [] + revoke: [] + revisions: + restore: null +observed: {} +``` + +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. + +`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. + +Executable contracts: +[core document](../packages/shared/src/agent-configuration.ts) and +[resource sections](../packages/shared/src/agent-configuration-resources.ts). +Resource inputs reuse the owning APIs' validators, including trust-policy, +secret, channel, skill, and tool validators. + +## Resource semantics + +### Instructions and skills + +`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. + +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 +contents over them. The native bundle service may seed an empty destination or +a missing entry file from the previous bundle. Review the freshly loaded files +before editing them. A managed root is assigned by Paperclip; choose `external` +mode to select another directory. + +Skills accept a key string or `{ key, versionId }`; `versionId: null` follows +the current version. An empty list unassigns skills through the normal sync +API. These keys are organization-library references, not embedded skill source. + +The `instructionsBundleMode`, `instructionsRootPath`, `instructionsEntryFile`, +`instructionsFilePath`, and `paperclipSkillSync` adapter keys are represented by +the dedicated sections. Direct edits to those adapter keys are rejected so two +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 +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. + +Each profile contains: + +```yaml +profileId: null # null creates a profile; an existing ID binds/edits it +updateSharedDefinition: false +priority: 100 +metadata: null +configuration: + profileKey: engineering + name: Engineering + description: null + status: active + defaultAction: deny + metadata: null + entries: + - selectorType: tool_name + effect: include + toolName: get_repository +``` + +The profile configuration uses the native profile/entry schema. Profiles can +be shared: editing a profile's definition affects every binding to that profile. +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`. + +Keep existing profile and policy fields explicit. Removing a field from an +existing definition is rejected because the native PATCH would otherwise +preserve its old value. Use the field's supported empty value to clear it. + +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`. + +### Channels + +Each `channels` row has immutable `id` and `provider`, plus `settings` and +`resources`. 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, +executionDefaults }`; removing a resource row disables it. Removing a channel +row removes the endpoint using its existing setup API. + +`channelOperations.create` accepts the native creation fields (`provider`, +optional `applicationId`, `name`, `slackApp`, `slackSetupMethod`); the current +agent ID is supplied by the editor. Save creation first to obtain its endpoint +ID before adding setup commands. `setup` accepts `{ id, input }`, where input +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`. + +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 +rejected before any writes. + +### Secrets, API keys, and revisions + +Use `secret_ref` and `user_secret_ref` bindings for existing credentials. +`secrets.create` accepts `{ secret, bind: { env: [], api: [] } }`; `secret` follows +the create-secret contract (name/key, provider, value or external reference, +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`. + +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 +creation and before the revision-guarded binding update. If creation succeeds +but binding fails, the receipt identifies the created secret; reload before +deciding how to bind it. + +`apiKeys.create` accepts `{ name, scope }`; scope uses the existing agent key +schema, with `{ kind: standard }` as the default. `apiKeys.revoke` contains key +IDs. Tokens returned by creation appear outside the YAML and are not stored in +the configuration query cache. Copy them before leaving the page. Previously +issued tokens cannot be retrieved. +Duplicate creation names within one save are rejected. + +`revisions.restore` accepts an existing revision ID or `null`. Restore must be +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, +terminate) are actions outside configuration. + +## YAML, secrets, and save guarantees + +The parser accepts one YAML 1.2 mapping containing JSON-compatible values. +Duplicate keys, multiple documents, aliases, anchors, explicit tags, merge +keys, non-string mapping keys, non-finite numbers, and prototype-related keys +are rejected. Documents are limited to 8 MiB and 50 levels of nesting. Syntax +errors include a location; schema errors identify a field path. There is no +interpolation, shell evaluation, include directive, or parser file access. +Comments and formatting are not persisted; saving/reloading regenerates YAML. +Changing only comments or mapping order does not write resources. +The editor shows **No configuration changes** and keeps Save disabled when the +operation plan is empty, including when an optional section is omitted. + +The initial core read checks configuration access before related resources are +loaded. Failed reads are shown as errors, never substituted with empty resource +lists. The editor validates document structure, known references, explicit +clears, and conflicting operations before enabling Save. Expand the change +summary to review the ordered operations, including removals and shared profile +edits. Domain-specific authorization, validation, and normalization still run +at each API write; not every server rejection can be predicted in the browser. + +Saving refreshes the snapshot before the first mutation. A changed core revision or an +edited resource section that has changed elsewhere rejects the save. Concurrent +changes to untouched resource sections are preserved. These are preflight +checks, not a transaction spanning all resources. + +Core changes use `PATCH /api/agents/:id` with +`expectedConfigurationRevision`, checked again under the row lock. Removing an +adapter key uses `replaceAdapterConfig: true`. The opaque revision uses a +domain-separated HMAC with the instance's private signing key. Credential edits +invalidate an open draft without exposing a public credential digest. Rotating +the signing key requires reloading open editors. Server processes for one +instance must share that key. File-based rotation takes effect on the next +revision calculation without a server restart. Managed instruction files and +entry revisions enforce hash/revision preconditions. External supporting file +writes/deletions do not enforce these preconditions. Other resources retain +their own API guarantees; changes between preflight and an unversioned resource +write are not atomically protected. Every mutation uses its existing company/actor authorization, +approval, validation, audit and revision behavior. + +A multi-resource save runs sequentially and is **not atomic**. On an interrupted +save, the editor retains the draft, reports completed steps and the failing +step, preserves any credential receipts, and disables Save until **Reload +saved**. Reload reveals current state and clears pending operations so retrying +cannot blindly duplicate a successful creation. A network failure may mean the +last operation committed even when its response did not arrive. No writes are +automatically retried. Double-clicking Save starts one operation, the editor +and Reload are disabled during saving, and agent navigation is held until it +settles. Dirty drafts use the agent page's unsaved-navigation guard. Closing the +browser cannot cancel or roll back requests already sent. + +A successful save reloads canonical state, clears command lists, and displays +any normalization warnings returned by the server. CEO agents and agents that +can create agents must explicitly retain task assignment authority. + +Stored inline credentials use `***REDACTED***`. An unchanged marker at its +original location preserves its saved value. Removing the key removes that +setting; replacing the full value supplies a new value under normal validation. +For revision-guarded core saves, the server restores unchanged masked subtrees +before validation; moved/partially edited redactions are rejected. Secret +references are never resolved into values. New secret/channel credential inputs +are one-time commands and disappear after canonical reload. This document is +an editable settings view, not a portable credential backup. + +No background polling, database migration, or new authorization bypass is +introduced by the editor. + +## Verification + +Run the browser acceptance suite against its disposable checkout instance: + +```sh +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 +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 +does not qualify provider login or agent execution. + +The lifecycle regressions can use a dedicated external PostgreSQL database when +embedded PostgreSQL is unavailable. Set +`PAPERCLIP_AGENT_LIFECYCLE_TEST_DATABASE_URL` to a database ending in `_test` +before running `server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts`. diff --git a/maps/agent-access-map/agents.md b/maps/agent-access-map/agents.md index 2fa84783c00..b3a2804d83d 100644 --- a/maps/agent-access-map/agents.md +++ b/maps/agent-access-map/agents.md @@ -108,6 +108,23 @@ qualification remains not run. ## `instructions` +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 +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 +unversioned resource writes retain a race window after preflight. Configuration reads include the complete redacted, revisioned fields +and `configurationRevision`; writes accept an optional +`expectedConfigurationRevision`, checked again under the write lock. A stale +write returns `409`. Revision-guarded writes preserve unchanged redacted values +at their original paths. Configuration read/configure authority, company scope, +instruction-path restrictions, environment/account checks, revision history, and +activity attribution still apply. YAML does not expose a permission-grant or +lifecycle mutation shortcut. See [the format](../../doc/agent-configuration-yaml.md). + Capability ID: `agents/instructions`. Review: `baseline`. diff --git a/maps/feature-map/agents.md b/maps/feature-map/agents.md index 909e3d1e084..8b511c038fe 100644 --- a/maps/feature-map/agents.md +++ b/maps/feature-map/agents.md @@ -49,6 +49,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 +[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 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 +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. + For a low-trust agent, open **Permissions / Trust → Allowed projects**. An operator with agent-management permission can select additional projects and explicitly save them. Existing projects and other containment restrictions remain intact. diff --git a/packages/shared/src/agent-configuration-resources.ts b/packages/shared/src/agent-configuration-resources.ts new file mode 100644 index 00000000000..bc407fcc18c --- /dev/null +++ b/packages/shared/src/agent-configuration-resources.ts @@ -0,0 +1,59 @@ +import { z } from "zod"; +import { updateAgentPermissionsSchema, createAgentKeySchema } from "./validators/agent.js"; +import { agentDesiredSkillSelectionSchema, agentDesiredSkillEntrySchema } from "./validators/adapter-skills.js"; +import { createToolProfileWithEntriesSchema, createToolProfileEntryForProfileSchema, createToolPolicySchema } from "./validators/tool-access.js"; +import { chatProviderSchema, updateChatEndpointSchema, replaceChatEndpointResourcesSchema, createChatEndpointSchema, configureChatEndpointSchema } from "./validators/chat-channels.js"; +import { createSecretSchema, rotateSecretSchema, updateSecretSchema } from "./validators/secret.js"; + +const id = z.string().min(1); +const filePath = z.string().min(1).max(512).refine(value => !value.startsWith("/") && !value.includes("\\") && !value.split("/").some(part => part === ".." || part === "." || !part), "Use a relative file path without traversal."); + +export const agentConfigurationResourceShape = { + permissions: updateAgentPermissionsSchema.extend({ + canCreateSkills: z.boolean(), trustPreset: updateAgentPermissionsSchema.shape.trustPreset.unwrap(), + }).strict().optional(), + instructions: z.object({ + mode: z.enum(["managed", "external"]).nullable(), + rootPath: z.string().nullable(), + entryFile: filePath, + files: z.record(filePath, z.string().max(1024 * 1024)), + }).strict().optional(), + skills: z.array(z.union([agentDesiredSkillSelectionSchema.options[0], agentDesiredSkillEntrySchema.strict()])).optional(), + tools: z.object({ + connections: z.array(z.object({ connectionId: id, installed: z.boolean() }).strict()), + profiles: z.array(z.object({ + profileId: id.nullable(), updateSharedDefinition: z.boolean().optional(), priority: z.number().int().min(0).max(10000), + metadata: z.record(z.string(), z.unknown()).nullable(), + configuration: createToolProfileWithEntriesSchema.extend({ + entries: z.array(createToolProfileEntryForProfileSchema.strict()).max(250).optional(), + }).strict(), + }).strict()), + policies: z.array(z.object({ id: id.nullable(), configuration: createToolPolicySchema.strict() }).strict()), + }).strict().optional(), + channels: z.array(z.object({ + id, provider: chatProviderSchema, + settings: updateChatEndpointSchema, + resources: z.array(replaceChatEndpointResourcesSchema.shape.resources.element.strict()), + }).strict()).optional(), + apiKeys: z.object({ create: z.array(createAgentKeySchema.strict()), revoke: z.array(id) }).strict().optional(), + revisions: z.object({ restore: id.nullable() }).strict().optional(), + secrets: z.object({ + create: z.array(z.object({ + secret: createSecretSchema.strict(), + bind: z.object({ env: z.array(z.string().regex(/^[A-Za-z_][A-Za-z0-9_]*$/)), api: z.array(z.string().regex(/^[A-Za-z_][A-Za-z0-9_]*$/)) }).strict(), + }).strict()), + rotate: z.array(z.object({ id, input: rotateSecretSchema.strict() }).strict()), + update: z.array(z.object({ id, input: updateSecretSchema.strict() }).strict()), + proposals: z.array(z.object({ id, decision: z.enum(["approve", "reject"]), reason: z.string().optional() }).strict()), + }).strict().optional(), + channelOperations: z.object({ + create: z.array(z.object(createChatEndpointSchema.shape).omit({ assignedAgentId: true }).strict().refine( + value => (!value.slackApp && !value.slackSetupMethod) || value.provider === "slack", + { message: "Slack app details only apply to Slack connections", path: ["slackApp"] }, + )), + setup: z.array(z.object({ id, input: configureChatEndpointSchema }).strict()), + revokeIdentityLinks: z.array(z.object({ endpointId: id, principalId: id }).strict()), + }).strict().optional(), +}; + +export type AgentConfigurationResources = z.infer>; diff --git a/packages/shared/src/agent-configuration.ts b/packages/shared/src/agent-configuration.ts new file mode 100644 index 00000000000..fad56b27a9a --- /dev/null +++ b/packages/shared/src/agent-configuration.ts @@ -0,0 +1,62 @@ +import { z } from "zod"; +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"; + +export const agentConfigurationSpecSchema = objectWithoutDefaults(createAgentSchema.pick({ + name: true, + role: true, + title: true, + icon: true, + appearance: true, + reportsTo: true, + capabilities: true, + adapterType: true, + adapterConfig: true, + runtimeConfig: true, + defaultEnvironmentId: true, + budgetMonthlyCents: true, + metadata: true, +})).required().strict(); + +export const agentConfigurationDocumentSchema = z.object({ + apiVersion: z.literal("paperclip/v1"), + kind: z.literal("Agent"), + spec: agentConfigurationSpecSchema.extend(agentConfigurationResourceShape), + observed: z.record(z.string(), z.unknown()).optional(), +}).strict(); + +export type AgentConfigurationSpec = z.infer; +export type AgentConfigurationDocument = z.infer; +export type AgentConfigurationSource = Omit, "role" | "adapterType"> & { + role: string; + adapterType: string; +}; +export type AgentConfigurationResponse = AgentConfigurationSource & { configurationRevision: string }; + +export function agentConfigurationSpec(agent: AgentConfigurationSource) { + return { + name: agent.name, + role: agent.role, + title: agent.title, + icon: agent.icon, + appearance: resolveAgentAppearance(agent.appearance, agent.id), + reportsTo: agent.reportsTo, + capabilities: agent.capabilities, + adapterType: agent.adapterType, + adapterConfig: agent.adapterConfig, + runtimeConfig: agent.runtimeConfig, + defaultEnvironmentId: agent.defaultEnvironmentId ?? null, + budgetMonthlyCents: agent.budgetMonthlyCents, + metadata: agent.metadata, + }; +} + +export function canonicalAgentConfiguration(value: unknown): string { + return JSON.stringify(value, (_key, item: unknown) => { + if (!item || typeof item !== "object" || Array.isArray(item)) return item; + return Object.fromEntries(Object.entries(item).sort(([a], [b]) => a < b ? -1 : a > b ? 1 : 0)); + }); +} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 2cac341fa39..6b149bc00e6 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -2851,3 +2851,4 @@ export * from "./prompt-injection-prevention.js"; export * from "./workspace-provider.js"; export * from "./workflows.js"; +export * from "./agent-configuration.js"; diff --git a/packages/shared/src/validators/agent.ts b/packages/shared/src/validators/agent.ts index 1e9e42e2876..4801092351f 100644 --- a/packages/shared/src/validators/agent.ts +++ b/packages/shared/src/validators/agent.ts @@ -174,6 +174,7 @@ export const updateAgentSchema = objectWithoutDefaults( permissions: z.never().optional(), runtimeSettings: agentRuntimeSettingsPatchSchema.optional(), replaceAdapterConfig: z.boolean().optional(), + expectedConfigurationRevision: z.string().regex(/^[a-f0-9]{64}$/).optional(), status: z.enum(["paused", "idle", "terminated"]).optional(), spentMonthlyCents: z.never().optional(), }); diff --git a/server/src/__tests__/agent-adapter-validation-routes.test.ts b/server/src/__tests__/agent-adapter-validation-routes.test.ts index 22515f7dc9c..4f0bc522188 100644 --- a/server/src/__tests__/agent-adapter-validation-routes.test.ts +++ b/server/src/__tests__/agent-adapter-validation-routes.test.ts @@ -334,6 +334,53 @@ describe("agent routes adapter validation", () => { await unregisterTestAdapter(missingAdapterType); }); + it("round-trips the protected YAML projection and forwards its revision to the write lock", async () => { + const existing = { + ...(await mockAgentService.getById()), + adapterConfig: { model: "old", removeMe: true, env: { DEMO_VALUE: "private-fixture" } }, + metadata: { token: "private-metadata-fixture" }, + }; + mockAgentService.getById.mockResolvedValue(existing); + const app = await createApp(); + const loaded = await requestApp(app, base => request(base).get(`/api/agents/${existing.id}/configuration`)); + expect(loaded.status).toBe(200); + expect(loaded.body.configurationRevision).toMatch(/^[a-f0-9]{64}$/); + expect(JSON.stringify(loaded.body)).not.toContain("private-fixture"); + expect(JSON.stringify(loaded.body)).not.toContain("private-metadata-fixture"); + const adapterConfig = { ...loaded.body.adapterConfig, model: "new" }; + delete adapterConfig.removeMe; + const saved = await requestApp(app, base => request(base).patch(`/api/agents/${existing.id}`).send({ + adapterConfig, replaceAdapterConfig: true, metadata: { ...loaded.body.metadata, label: "new" }, + expectedConfigurationRevision: loaded.body.configurationRevision, + })); + expect(saved.status, JSON.stringify(saved.body)).toBe(200); + expect(mockAgentService.update).toHaveBeenCalledWith(existing.id, expect.objectContaining({ + adapterConfig: expect.objectContaining({ model: "new", env: existing.adapterConfig.env }), + metadata: { ...existing.metadata, label: "new" }, + }), expect.objectContaining({ expectedConfigurationRevision: loaded.body.configurationRevision })); + expect(mockAgentService.update.mock.calls[0][1].adapterConfig).not.toHaveProperty("removeMe"); + expect(saved.body.configurationRevision).not.toBe(loaded.body.configurationRevision); + expect(JSON.stringify(saved.body)).not.toContain("private-fixture"); + expect(JSON.stringify(saved.body)).not.toContain("private-metadata-fixture"); + expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ action: "agent.updated" })); + }); + + it("rejects stale and unauthorized YAML saves before calling the update service", async () => { + const existing = await mockAgentService.getById(); + const app = await createApp(); + const loaded = await requestApp(app, base => request(base).get(`/api/agents/${existing.id}/configuration`)); + const stale = await requestApp(app, base => request(base).patch(`/api/agents/${existing.id}`).send({ + title: "Stale", expectedConfigurationRevision: "0".repeat(64), + })); + expect(stale.status).toBe(409); + mockAccessService.decide.mockResolvedValue({ allowed: false, reason: "deny_missing_grant", explanation: "No configuration permission" }); + const denied = await requestApp(app, base => request(base).patch(`/api/agents/${existing.id}`).send({ + title: "Denied", expectedConfigurationRevision: loaded.body.configurationRevision, + })); + expect(denied.status).toBe(403); + expect(mockAgentService.update).not.toHaveBeenCalled(); + }); + it("routes narrow roster edits through authorization, config merging and audit history", async () => { const existing = { ...(await mockAgentService.getById()), adapterConfig: { model: "old", cwd: "/workspace" }, runtimeConfig: { heartbeat: { intervalSec: 900 } } }; mockAgentService.getById.mockResolvedValue(existing); diff --git a/server/src/__tests__/agent-configuration-revision.test.ts b/server/src/__tests__/agent-configuration-revision.test.ts new file mode 100644 index 00000000000..2eacda7c437 --- /dev/null +++ b/server/src/__tests__/agent-configuration-revision.test.ts @@ -0,0 +1,67 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { resolveAgentAppearance } from "@paperclipai/shared"; +import { agentConfigurationRevision, restoreAgentConfigurationRedactions } from "../lib/agent-configuration-revision.js"; +import { redactAgentAdapterConfig } from "../redaction.js"; +import { resolveDefaultSecretsKeyFilePath } from "../home-paths.js"; + +const agent = { + id: "11111111-1111-4111-8111-111111111111", name: "Codie", role: "engineer" as const, title: null, + icon: null, appearance: null, reportsTo: null, capabilities: null, adapterType: "codex_local" as const, + adapterConfig: { model: "example", env: { API_KEY: "fake-secret" }, apiKey: "fake-adapter-secret" }, + runtimeConfig: {}, defaultEnvironmentId: null, budgetMonthlyCents: 0, metadata: null, permissions: {}, +}; + +describe("agent configuration revisions and redactions", () => { + afterEach(() => vi.unstubAllEnvs()); + + it("keys revisions with the private instance secret while detecting hidden credential edits", () => { + vi.stubEnv("PAPERCLIP_DECISION_SIGNING_SECRET", "a".repeat(32)); + const original = agentConfigurationRevision(agent); + expect(agentConfigurationRevision({ ...agent, adapterConfig: { ...agent.adapterConfig, apiKey: "changed-hidden-secret" } })) + .not.toBe(original); + vi.stubEnv("PAPERCLIP_DECISION_SIGNING_SECRET", "b".repeat(32)); + expect(agentConfigurationRevision(agent)).not.toBe(original); + vi.stubEnv("PAPERCLIP_DECISION_SIGNING_SECRET", "a".repeat(32)); + expect(agentConfigurationRevision(agent)).toBe(original); + }); + + it("uses a signing key rotated in place without restarting the process", () => { + const home = mkdtempSync(path.join(os.tmpdir(), "agent-yaml-signing-")); + vi.stubEnv("PAPERCLIP_HOME", home); + vi.stubEnv("PAPERCLIP_DECISION_SIGNING_SECRET", ""); + try { + const original = agentConfigurationRevision(agent); + const keyPath = path.join(path.dirname(resolveDefaultSecretsKeyFilePath()), "decision-signing.key"); + const previous = readFileSync(keyPath, "utf8"); + writeFileSync(keyPath, "rotated-private-key".repeat(4), { mode: 0o600 }); + expect(agentConfigurationRevision(agent)).not.toBe(original); + writeFileSync(keyPath, previous, { mode: 0o600 }); + expect(agentConfigurationRevision(agent)).toBe(original); + } finally { rmSync(home, { recursive: true, force: true }); } + }); + + it("normalizes raw and presented rows and ignores live bookkeeping", () => { + expect(agentConfigurationRevision({ ...agent, appearance: resolveAgentAppearance(null, agent.id), permissions: { canCreateAgents: false, canCreateSkills: true } })) + .toBe(agentConfigurationRevision(agent)); + const runtimeChange = { ...agent, status: "running", spentMonthlyCents: 100, updatedAt: new Date() }; + expect(agentConfigurationRevision(runtimeChange)).toBe(agentConfigurationRevision(agent)); + for (const patch of [{ name: "Renamed" }, { budgetMonthlyCents: 2 }, { adapterConfig: { ...agent.adapterConfig, model: "new" } }]) { + expect(agentConfigurationRevision({ ...agent, ...patch })).not.toBe(agentConfigurationRevision(agent)); + } + }); + + it("preserves redacted secrets while changing another key and permits explicit removal", () => { + const displayed = redactAgentAdapterConfig(agent.adapterConfig); + expect(restoreAgentConfigurationRedactions({ ...displayed, model: "new" }, agent.adapterConfig, displayed)) + .toEqual({ ...agent.adapterConfig, model: "new" }); + expect(restoreAgentConfigurationRedactions({ model: "new" }, agent.adapterConfig, displayed)).toEqual({ model: "new" }); + }); + + it("rejects moved and partially edited redactions", () => { + expect(() => restoreAgentConfigurationRedactions({ other: "***REDACTED***" }, { apiKey: "secret" }, { apiKey: "***REDACTED***" })).toThrow("redacted value"); + expect(() => restoreAgentConfigurationRedactions("Bearer ***REDACTED*** edited", "Bearer secret", "Bearer ***REDACTED***")).toThrow("redacted value"); + }); +}); diff --git a/server/src/lib/agent-configuration-revision.ts b/server/src/lib/agent-configuration-revision.ts new file mode 100644 index 00000000000..89ebd49c64b --- /dev/null +++ b/server/src/lib/agent-configuration-revision.ts @@ -0,0 +1,32 @@ +import { createHmac } from "node:crypto"; +import { agentConfigurationSpec, canonicalAgentConfiguration, type AgentConfigurationSource } from "@paperclipai/shared"; +import { normalizeAgentPermissions } from "./agent-permissions.js"; +import { REDACTED_EVENT_VALUE } from "../redaction.js"; +import { unprocessable } from "../errors.js"; +import { resolveDecisionSigningSecret } from "../services/decision-signing.js"; + +export function agentConfigurationRevision(agent: AgentConfigurationSource & { permissions: Record }) { + return createHmac("sha256", resolveDecisionSigningSecret()).update("agent-configuration-v1:").update(canonicalAgentConfiguration({ + id: agent.id, ...agentConfigurationSpec(agent), permissions: normalizeAgentPermissions(agent.permissions), + })).digest("hex"); +} + +export function restoreAgentConfigurationRedactions(requested: unknown, saved: unknown, displayed: unknown): unknown { + if (canonicalAgentConfiguration(requested) === canonicalAgentConfiguration(displayed)) return saved; + if (typeof requested === "string" && requested.includes(REDACTED_EVENT_VALUE)) { + throw unprocessable("A redacted value was changed or moved. Keep it unchanged, remove it, or replace the entire value."); + } + if (Array.isArray(requested)) { + return requested.map((value, index) => restoreAgentConfigurationRedactions(value, + Array.isArray(saved) ? saved[index] : undefined, Array.isArray(displayed) ? displayed[index] : undefined)); + } + if (requested && typeof requested === "object") { + const previous = saved && typeof saved === "object" ? saved as Record : {}; + const redacted = displayed && typeof displayed === "object" ? displayed as Record : {}; + return Object.fromEntries(Object.entries(requested).map(([key, value]) => [key, + restoreAgentConfigurationRedactions(value, Object.hasOwn(previous, key) ? previous[key] : undefined, + Object.hasOwn(redacted, key) ? redacted[key] : undefined), + ])); + } + return requested; +} diff --git a/server/src/lib/agent-records.ts b/server/src/lib/agent-records.ts index 25c0ed7c9f5..c28eb81aa18 100644 --- a/server/src/lib/agent-records.ts +++ b/server/src/lib/agent-records.ts @@ -74,6 +74,7 @@ export interface ClaudeLoginContext { } export interface UpdateAgentOptions { + expectedConfigurationRevision?: string; expectedConfiguration?: Pick; recordRevision?: RevisionMetadata; allowBuiltInAgentMetadata?: boolean; diff --git a/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts b/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts index 8f494cc6fa7..f6c92607fc5 100644 --- a/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts +++ b/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts @@ -16,7 +16,7 @@ import { errorHandler } from "../../../middleware/error-handler.js"; import { createLifecycleDriver } from "../../../services/agent-lifecycle-driver.js"; import { randomUUID } from "node:crypto"; import { and, eq, sql } from "drizzle-orm"; -import { agents, approvals, projects, issues, heartbeatRuns, nativeRunFinalizations, budgetPolicies, environmentLeases, environments, agentApiKeys, activityLog, agentConfigRevisions, userCompanyPreferences, companies, companyMemberships, principalPermissionGrants, plugins, pluginCompanySettings, createDb, type Db } from "@paperclipai/db"; +import { agents, approvals, projects, issues, heartbeatRuns, nativeRunFinalizations, budgetPolicies, environmentLeases, environments, agentApiKeys, activityLog, agentConfigRevisions, userCompanyPreferences, companies, companyMemberships, principalPermissionGrants, plugins, pluginCompanySettings, applyPendingMigrations, closeRegisteredClients, createDb, type Db } from "@paperclipai/db"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../../../__tests__/helpers/embedded-postgres.js"; import { createAgentLifecycle, configureAgentLifecycle, startAgentLifecycle } from "../../../services/agent-lifecycle.js"; @@ -25,17 +25,27 @@ import { createLifecycleStore as createStore } from "../adapters/postgres.js"; import { transition } from "../domain/policy.js"; import type { LifecycleAgent } from "../application/ports.js"; import { budgetService } from "../../../services/budgets.js"; +import { agentConfigurationRevision } from "../../../lib/agent-configuration-revision.js"; +import { resolveExternalTestDatabaseUrl } from "../../../__tests__/helpers/external-test-database.js"; const createLifecycleStore = (db: Db) => createStore(db, createAgentLifecycleEffects()); -const support = await getEmbeddedPostgresTestSupport(); +const external = resolveExternalTestDatabaseUrl("PAPERCLIP_AGENT_LIFECYCLE_TEST_DATABASE_URL"); +const support = external ? { supported: true } : await getEmbeddedPostgresTestSupport(); (support.supported ? describe : describe.skip)("agent lifecycle commands", () => { let database: Awaited>; let db: Db; let companyId: string; const workers: ReturnType[] = []; - beforeAll(async () => { database = await startEmbeddedPostgresTestDatabase("paperclip-agent-lifecycle-"); db = createDb(database.connectionString); }, 90_000); - afterAll(async () => { for (const worker of workers) await worker.stop(); await database?.cleanup(); }); + beforeAll(async () => { + if (external) { await applyPendingMigrations(external); db = createDb(external); } + else { database = await startEmbeddedPostgresTestDatabase("paperclip-agent-lifecycle-"); db = createDb(database.connectionString); } + }, 90_000); + afterAll(async () => { + for (const worker of workers) await worker.stop(); + if (external) await closeRegisteredClients(external); + else await database?.cleanup(); + }); beforeEach(async () => { for (const work of workers) await work.stop(); companyId = randomUUID(); @@ -75,6 +85,53 @@ const support = await getEmbeddedPostgresTestSupport(); expect((await current(agent.id)).runtimeConfig).toEqual({ heartbeat: { intervalSec: 42 } }); }); + it("fences YAML saves under the write lock without treating runtime bookkeeping as an edit", async () => { + const agent = await hire(); + const revision = agentConfigurationRevision(await current(agent.id)); + await db.update(agents).set({ lastHeartbeatAt: new Date(), updatedAt: new Date() }).where(eq(agents.id, agent.id)); + await createAgentLifecycle(db).updateConfiguration(agent.id, { title: "New title" }, { expectedConfigurationRevision: revision }); + await expect(createAgentLifecycle(db).updateConfiguration(agent.id, { name: "Stale overwrite" }, { expectedConfigurationRevision: revision })) + .rejects.toThrow("configuration changed while you were editing"); + expect(await current(agent.id)).toMatchObject({ name: agent.name, title: "New title" }); + }); + + it("saves YAML configuration through the normal route with redaction, revisions, and access checks", async () => { + const [agent] = await db.insert(agents).values({ companyId, name: "YAML test", adapterType: "process", + adapterConfig: { command: "echo", removeMe: true, env: { DEMO_VALUE: "private-fixture" }, apiKey: "private-api-fixture" }, + runtimeConfig: { heartbeat: { enabled: false } }, metadata: { token: "private-metadata-fixture" }, + }).returning(); + function app(actor: Express.Request["actor"]) { + const app = express(); app.use(express.json()); + app.use((req, _res, next) => { req.actor = actor; next(); }); + app.use("/api", agentRoutes(db)); app.use(errorHandler); return app; + } + const board = app({ type: "board", source: "local_implicit", userId: "board", companyIds: [companyId] }); + const response = await request(board).get(`/api/agents/${agent.id}/configuration`).expect(200); + expect(response.body).toMatchObject({ capabilities: null, budgetMonthlyCents: 0, defaultEnvironmentId: null }); + expect(JSON.stringify(response.body)).not.toContain("private-fixture"); + expect(JSON.stringify(response.body)).not.toContain("private-api-fixture"); + expect(JSON.stringify(response.body)).not.toContain("private-metadata-fixture"); + const adapterConfig = { ...response.body.adapterConfig, command: "printf" }; + delete adapterConfig.removeMe; + const saved = await request(board).patch(`/api/agents/${agent.id}`).send({ + adapterConfig, replaceAdapterConfig: true, title: "Updated using YAML", + expectedConfigurationRevision: response.body.configurationRevision, + }); + expect(saved.status, JSON.stringify(saved.body)).toBe(200); + expect(saved.body.configurationRevision).not.toBe(response.body.configurationRevision); + const persisted = await current(agent.id); + expect(persisted.adapterConfig).toMatchObject({ command: "printf", env: { DEMO_VALUE: { type: "plain", value: "private-fixture" } }, apiKey: "private-api-fixture" }); + expect(persisted.adapterConfig).not.toHaveProperty("removeMe"); + expect(persisted.runtimeConfig).toEqual(agent.runtimeConfig); + expect(persisted.metadata).toEqual(agent.metadata); + expect(await db.select().from(agentConfigRevisions).where(eq(agentConfigRevisions.agentId, agent.id))).toHaveLength(1); + expect(await db.select().from(activityLog).where(and(eq(activityLog.entityId, agent.id), eq(activityLog.action, "agent.updated")))).toHaveLength(1); + await request(board).patch(`/api/agents/${agent.id}`).send({ name: "Stale", expectedConfigurationRevision: response.body.configurationRevision }).expect(409); + const foreign = app({ type: "board", source: "session", userId: "outsider", companyIds: [], memberships: [] }); + await request(foreign).get(`/api/agents/${agent.id}/configuration`).expect(403); + await request(foreign).patch(`/api/agents/${agent.id}`).send({ title: "Denied", expectedConfigurationRevision: saved.body.configurationRevision }).expect(404); + }); + it("maps existing states without starting setup or clearing pause reasons", async () => { const rows = await db.insert(agents).values(["idle", "running", "error", "paused", "pending_approval", "terminated"].map(status => ({ companyId, name: status, status, pauseReason: status === "paused" ? "budget" : null, diff --git a/server/src/modules/agent-lifecycle/adapters/configuration.ts b/server/src/modules/agent-lifecycle/adapters/configuration.ts index 011508a0d78..7359ddfacf1 100644 --- a/server/src/modules/agent-lifecycle/adapters/configuration.ts +++ b/server/src/modules/agent-lifecycle/adapters/configuration.ts @@ -1,5 +1,6 @@ import { isDeepStrictEqual } from "node:util"; import { conflict } from "../../../errors.js"; +import { agentConfigurationRevision } from "../../../lib/agent-configuration-revision.js"; import { randomUUID } from "node:crypto"; import { eq } from "drizzle-orm"; import { agents, type Db } from "@paperclipai/db"; @@ -13,6 +14,9 @@ export async function updateAgentConfiguration(tx: Db, effects: LifecycleEffects assertConfigurationFields(data); const [current] = await tx.select().from(agents).where(eq(agents.id, id)).for("no key update"); if (!current) return null; + if (options?.expectedConfigurationRevision && options.expectedConfigurationRevision !== agentConfigurationRevision(current)) { + throw conflict("Agent configuration changed while you were editing. Reload the saved configuration and apply your changes again."); + } if (options?.expectedConfiguration && Object.entries(options.expectedConfiguration).some(([key, value]) => !isDeepStrictEqual(current[key as keyof typeof current], value))) { throw conflict("Agent configuration changed while saving. Refresh the roster and review the changes again."); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index d81f90d21cb..5e066f8f0ba 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1,4 +1,6 @@ import { latestFailedRunsByAgent } from "@paperclipai/shared"; +import { agentConfigurationSpec } from "@paperclipai/shared"; +import { agentConfigurationRevision, restoreAgentConfigurationRedactions } from "../lib/agent-configuration-revision.js"; import { prepareRuntimeSettingsPatch } from "../services/agent-runtime-settings.js"; import { completeSetupTokenAiConnection } from "../services/setup-token-ai-connection.js"; import { connectionManagementService, continuingConnectionManagement } from "../services/connection-management.js"; @@ -2943,6 +2945,9 @@ export function agentRoutes( function redactAgentConfiguration(agent: Awaited>) { if (!agent) return null; return { + ...agentConfigurationSpec(agent), + configurationRevision: agentConfigurationRevision(agent), + metadata: redactEventPayload(agent.metadata), id: agent.id, companyId: agent.companyId, name: agent.name, @@ -5399,6 +5404,20 @@ export function agentRoutes( : { ...(req.body as Record) }; const runner = patchData.runner as AgentRunnerChoice | undefined; delete patchData.runner; + const expectedConfigurationRevision = patchData.expectedConfigurationRevision as string | undefined; + delete patchData.expectedConfigurationRevision; + if (expectedConfigurationRevision && expectedConfigurationRevision !== agentConfigurationRevision(existing)) { + throw conflict("Agent configuration changed while you were editing. Reload the saved configuration and apply your changes again."); + } + if (expectedConfigurationRevision) { + await assertCanConfigureAgentSetup(req, existing); + const displayed = redactAgentConfiguration(existing)!; + for (const key of ["adapterConfig", "runtimeConfig", "metadata"] as const) { + if (hasOwn(patchData, key)) { + patchData[key] = restoreAgentConfigurationRedactions(patchData[key], existing[key], displayed[key]); + } + } + } const requestedStatus = patchData.status; delete patchData.status; if (requestedStatus !== undefined && !["paused", "idle", "terminated"].includes(String(requestedStatus))) throw badRequest("Use an agent lifecycle command to change status"); @@ -5651,6 +5670,7 @@ export function agentRoutes( const actor = getActorInfo(req); const updateOptions = { + expectedConfigurationRevision, ...(runtimeSettings ? { expectedConfiguration: { adapterType: existing.adapterType, adapterConfig: existing.adapterConfig, runtimeConfig: existing.runtimeConfig, defaultEnvironmentId: existing.defaultEnvironmentId, @@ -5690,7 +5710,7 @@ export function agentRoutes( details: summarizeAgentUpdateDetails(patchData), }); - res.json({ ...redactAgentRowForResponse(agent), ...(configurationWarnings.length ? { configurationWarnings } : {}) }); + res.json({ ...redactAgentRowForResponse(agent), ...redactAgentConfiguration(agent), ...(configurationWarnings.length ? { configurationWarnings } : {}) }); }); router.post("/agents/:id/lifecycle/retry", async (req, res) => { diff --git a/tests/e2e/agent-configuration.spec.ts b/tests/e2e/agent-configuration.spec.ts new file mode 100644 index 00000000000..d09b17dc864 --- /dev/null +++ b/tests/e2e/agent-configuration.spec.ts @@ -0,0 +1,241 @@ +import { mkdtemp, readFile, writeFile } from "node:fs/promises"; +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 { startReviewProvider } from "../fixtures/connection-review-provider"; + +const test = base.extend<{ toolProvider: Awaited>; settings: void }>({ + settings: [async ({ request }, use) => { + const previous = await json(await request.get("/api/instance/settings/experimental")); + try { + await json(await request.patch("/api/instance/settings/experimental", { data: { enableBetaSkills: true, enableChatConnectors: true } })); + await use(); + } finally { + await json(await request.patch("/api/instance/settings/experimental", { data: { + enableBetaSkills: previous.enableBetaSkills, enableChatConnectors: previous.enableChatConnectors, + } })); + } + }, { auto: true }], + toolProvider: async ({}, use) => { + const provider = await startReviewProvider(); + try { await use(provider); } + finally { await provider.close(); } + }, +}); + +test.use({ permissions: ["clipboard-read", "clipboard-write"] }); + +async function json(response: APIResponse) { + expect(response.ok(), `${response.status()}: ${await response.text()}`).toBe(true); + return response.json(); +} + +async function setup(request: APIRequestContext) { + const company = await json(await request.post("/api/companies", { data: { name: `Agent YAML ${Date.now()}` } })); + const agent = await json(await request.post(`/api/companies/${company.id}/agents`, { data: { + name: "YAML engineer", role: "engineer", adapterType: "process", adapterConfig: { command: "true", removable: "old value" }, + runtimeConfig: { heartbeat: { enabled: false, wakeOnDemand: false } }, + } })); + return { company, agent, url: `/${company.issuePrefix}/agents/${agent.urlKey}` }; +} + +const editor = (page: Page) => page.getByRole("textbox", { name: "Agent YAML configuration" }); +async function document(page: Page): Promise { + await expect(editor(page)).toBeVisible(); + await expect(editor(page)).toHaveAttribute("contenteditable", "true"); + await editor(page).press("ControlOrMeta+a"); + await editor(page).press("ControlOrMeta+c"); + const source = await page.evaluate(() => navigator.clipboard.readText()); + expect(source).toContain("apiVersion: paperclip/v1"); + return parse(source); +} +async function edit(page: Page, change: (value: AgentConfigurationDocument) => 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"); +} +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: "Reload saved", exact: true })).toBeEnabled(); + await expect(page.getByRole("alert")).toHaveCount(0); +} + +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); + await json(await request.patch(`/api/agents/${agent.id}/instructions-bundle`, { data: { mode: "managed" } })); + const skill = await json(await request.post(`/api/companies/${company.id}/skills`, { data: { + name: "YAML proof", slug: "yaml-proof", markdown: "---\nname: yaml-proof\ndescription: Verify saved configuration.\n---\nCheck the persisted state.\n", + } })); + const connection = await json(await request.post(`/api/companies/${company.id}/tools/connections`, { data: { + name: "YAML tools", transport: "mcp_remote", config: { url: toolProvider.url }, status: "draft", enabled: false, + } })); + await page.goto(`${url}/overview`); + await page.getByRole("link", { name: "Advanced", exact: true }).click(); + 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"; + value.spec.skills = [{ key: skill.key, versionId: skill.currentVersionId }]; + value.spec.tools!.connections.find(row => row.connectionId === connection.id)!.installed = true; + 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" }]; + }); + await save(page); + const token = page.getByRole("textbox", { name: "API key: YAML automation token" }); + await expect(token).toBeVisible(); + expect(await token.inputValue()).toMatch(/^pcp_/); + await page.reload(); + 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(stringify(saved)).not.toContain("disposable-yaml-secret"); + expect(saved.spec.instructions!.files["workflow.md"]).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!.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((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.tools!.profiles = []; + value.spec.tools!.policies = []; + value.spec.tools!.connections.find(row => row.connectionId === connection.id)!.installed = false; + 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(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(); + await page.screenshot({ path: info.outputPath("saved-agent-yaml.png") }); +}); + +test("stale drafts and invalid commands leave saved configuration unchanged", async ({ page, request }) => { + const { agent, url } = await setup(request); + await page.goto(`${url}/advanced`); + await edit(page, value => { value.spec.title = "Local draft"; }); + await json(await request.patch(`/api/agents/${agent.id}`, { data: { title: "Other operator" } })); + await page.getByRole("button", { name: "Save changes", exact: true }).click(); + await expect(page.getByRole("alert")).toContainText("changed while you were editing"); + expect((await document(page)).spec.title).toBe("Local draft"); + expect((await json(await request.get(`/api/agents/${agent.id}/configuration`))).title).toBe("Other operator"); + await page.getByRole("button", { name: "Reload saved", exact: true }).click(); + await expect.poll(async () => (await document(page)).spec.title).toBe("Other operator"); + await edit(page, value => { value.observed = {}; }); + await expect(page.getByRole("alert")).toContainText("observed is read-only"); + await expect(page.getByRole("button", { name: "Save changes", exact: true })).toBeDisabled(); +}); + +test("renaming an agent keeps newly created key tokens visible through the route change", async ({ page, request }) => { + const { agent, url } = await setup(request); + 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" } }]; + }); + await save(page); + await expect(page).toHaveURL(/\/agents\/renamed-yaml-engineer\/advanced$/); + const token = page.getByRole("textbox", { name: "API key: Rename key token" }); + await expect(token).toBeVisible(); + const created = await token.inputValue(); + expect(created).toMatch(/^pcp_/); + expect(stringify(await document(page))).not.toContain(created); + expect(await json(await request.get(`/api/agents/${agent.id}/keys`))).toHaveLength(1); + await edit(page, value => { value.spec.title = "Another saved edit"; }); + await save(page); + await expect(token).toHaveValue(created); +}); + +test("a lost creation response requires reload and cannot blindly replay commands", async ({ page, request }) => { + const { agent, url } = await setup(request); + let creates = 0; + await page.route(`**/api/agents/${agent.id}/keys?*`, async route => { + if (route.request().method() !== "POST") return route.continue(); + creates++; + if (creates === 1) return route.continue(); + await route.fetch(); + return route.abort("failed"); + }); + await page.goto(`${url}/advanced`); + 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" } })); + }); + await page.getByRole("button", { name: "Save changes", exact: true }).click(); + await expect(page.getByRole("alert")).toContainText("Reload saved before saving again"); + await expect(page.getByRole("textbox", { name: "API key: First key token" })).toBeVisible(); + await expect(page.getByRole("button", { name: "Save changes", exact: true })).toBeDisabled(); + expect((await json(await request.get(`/api/agents/${agent.id}/configuration`))).title).toBe("Saved before lost response"); + expect(await json(await request.get(`/api/agents/${agent.id}/keys`))).toHaveLength(2); + expect((await document(page)).spec.apiKeys!.create).toHaveLength(2); + await page.getByRole("button", { name: "Reload saved", exact: true }).click(); + await expect.poll(async () => (await document(page)).spec.apiKeys!.create).toEqual([]); + await expect(page).toHaveURL(/\/agents\/partially-renamed-yaml-engineer\/advanced$/); + await expect(page.getByRole("textbox", { name: "API key: First key token" })).toBeVisible(); + await edit(page, value => { value.spec.title = "Recovered save"; }); + await save(page); + expect(creates).toBe(2); + expect(await json(await request.get(`/api/agents/${agent.id}/keys`))).toHaveLength(2); +}); + +test("switching instructions location loads existing destination files without overwriting them", async ({ page, request }) => { + const { agent, url } = await setup(request); + await json(await request.patch(`/api/agents/${agent.id}/instructions-bundle`, { data: { mode: "managed" } })); + const destination = await mkdtemp(path.join(os.tmpdir(), "agent-yaml-instructions-")); + await writeFile(path.join(destination, "AGENTS.md"), "Keep destination instructions.\n"); + 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 readFile(path.join(destination, "AGENTS.md"), "utf8")).toBe("Keep destination instructions.\n"); +}); + +test("revision restore is an explicit separate save and reloads the restored configuration", async ({ page, request }) => { + const { agent, url } = await setup(request); + await page.goto(`${url}/advanced`); + await edit(page, value => { value.spec.title = "Before restore"; }); + await save(page); + const revisions = await json(await request.get(`/api/agents/${agent.id}/config-revisions`)); + const revision = revisions.find((row: { changedKeys: string[] }) => row.changedKeys.includes("title")); + 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 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"; }); + await save(page); + await page.reload(); + const restored = await document(page); + expect(restored.spec.title).toBe("Before restore"); + expect(restored.spec.revisions!.restore).toBeNull(); +}); diff --git a/ui/package.json b/ui/package.json index 279b2553b14..e59f87e405d 100644 --- a/ui/package.json +++ b/ui/package.json @@ -31,10 +31,15 @@ "dependencies": { "@assistant-ui/react": "0.15.22", "@base-ui/react": "^1.7.0", + "@codemirror/lang-yaml": "^6.1.3", + "@codemirror/language": "^6.12.4", + "@codemirror/state": "^6.7.2", + "@codemirror/view": "^6.43.11", "@dnd-kit/core": "^6.3.1", "@dnd-kit/sortable": "^10.0.0", "@dnd-kit/utilities": "^3.2.2", "@lexical/link": "0.48.0", + "@lezer/highlight": "^1.2.3", "@mdxeditor/editor": "^4.2.3", "@paperclipai/adapter-claude-local": "workspace:*", "@paperclipai/adapter-codex-local": "workspace:*", @@ -58,6 +63,7 @@ "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "cmdk": "^1.1.1", + "codemirror": "^6.0.2", "i18next": "^26.4.2", "lexical": "0.48.0", "lucide-react": "^1.48.0", @@ -72,6 +78,7 @@ "react-router-dom": "^7.18.4", "remark-gfm": "^4.0.1", "tailwind-merge": "^3.6.0", + "yaml": "^2.8.1", "yjs": "13.6.32" }, "devDependencies": { diff --git a/ui/src/api/agents.ts b/ui/src/api/agents.ts index 9f97ce68bfe..a20eb913bab 100644 --- a/ui/src/api/agents.ts +++ b/ui/src/api/agents.ts @@ -154,7 +154,7 @@ export const agentsApi = { } }, getConfiguration: (id: string, companyId?: string) => - api.get>(agentPath(id, companyId, "/configuration")), + api.get(agentPath(id, companyId, "/configuration")), listConfigRevisions: (id: string, companyId?: string) => api.get(agentPath(id, companyId, "/config-revisions")), getConfigRevision: (id: string, revisionId: string, companyId?: string) => @@ -168,7 +168,7 @@ export const agentsApi = { updateRuntimeSettings: (id: string, runtimeSettings: import("@paperclipai/shared").AgentRuntimeSettingsPatch, companyId: string) => api.patch(agentPath(id, companyId), { runtimeSettings }), update: (id: string, data: Record, companyId?: string) => - api.patch(agentPath(id, companyId), data), + api.patch(agentPath(id, companyId), data), updatePermissions: (id: string, data: AgentPermissionUpdate, companyId?: string) => api.patch(agentPath(id, companyId, "/permissions"), data), instructionsBundle: (id: string, companyId?: string) => diff --git a/ui/src/components/AgentContextualSidebar.test.tsx b/ui/src/components/AgentContextualSidebar.test.tsx index 50cfae97ab0..8641ef1c6a3 100644 --- a/ui/src/components/AgentContextualSidebar.test.tsx +++ b/ui/src/components/AgentContextualSidebar.test.tsx @@ -63,6 +63,7 @@ describe("AgentContextualSidebar", () => { expect(markup).toContain("border-r border-border bg-background"); expect(markup).toContain('data-slot="contextual-sidebar-nav"'); expect(markup).toContain('href="/agents/codexcoder/overview"'); + expect(markup).toContain('href="/agents/codexcoder/advanced"'); expect(markup).toContain('href="/agents/codexcoder/permissions"'); expect(markup).toContain('href="/agents/codexcoder/api-keys"'); expect(markup).toContain('href="/activity?mode=agents&agentId=agent-1"'); diff --git a/ui/src/components/AgentContextualSidebar.tsx b/ui/src/components/AgentContextualSidebar.tsx index 78e1838c488..40b7b470d09 100644 --- a/ui/src/components/AgentContextualSidebar.tsx +++ b/ui/src/components/AgentContextualSidebar.tsx @@ -3,6 +3,7 @@ import { Activity, BadgeDollarSign, BookOpenText, + FileCode2, History, KeyRound, Library, @@ -39,6 +40,7 @@ const localIcons = { permissions: ShieldCheck, "api-keys": KeyRound, revisions: History, + advanced: FileCode2, } satisfies Record; const auditItems = [ diff --git a/ui/src/index.css b/ui/src/index.css index 04b918dd2dd..b1008fe998e 100644 --- a/ui/src/index.css +++ b/ui/src/index.css @@ -3295,6 +3295,26 @@ span.paperclip-mention-chip[data-mention-kind="external-object"] { ── end allowlist ── */ /* Agent settings composition, shared with Storybook. */ +.agent-yaml-editor .cm-editor { + height: var(--sz-480px); + font-family: var(--font-mono); + font-size: var(--text-sm); + background: var(--background); + color: var(--foreground); +} +.agent-yaml-editor .cm-scroller { overflow: auto; } +.agent-yaml-editor .cm-gutters { + background: var(--muted); + color: var(--muted-foreground); + border-color: var(--border); +} +.agent-yaml-editor .cm-activeLine, +.agent-yaml-editor .cm-activeLineGutter { background: var(--accent); } +.agent-yaml-editor .cm-cursor { border-left-color: var(--foreground); } +.agent-yaml-editor .cm-selectionBackground, +.agent-yaml-editor .cm-focused .cm-selectionBackground { background: var(--accent); } +.agent-yaml-editor .cm-focused { outline-color: var(--ring); } + .agent-settings-form > .relative { @apply flex flex-col gap-8; } diff --git a/ui/src/lib/agent-configuration-yaml.test.ts b/ui/src/lib/agent-configuration-yaml.test.ts new file mode 100644 index 00000000000..0a65d0d4b11 --- /dev/null +++ b/ui/src/lib/agent-configuration-yaml.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; +import { appearanceForPalette, type AgentConfigurationSource } from "@paperclipai/shared"; +import { agentConfigurationPatch, parseAgentConfigurationYaml, serializeAgentConfiguration } from "./agent-configuration-yaml"; + +export const configuration: AgentConfigurationSource = { + id: "11111111-1111-4111-8111-111111111111", name: "Codie", role: "engineer", title: null, + icon: null, appearance: appearanceForPalette("bubblegum-sky"), reportsTo: null, + capabilities: "Write code\nReview changes\n", adapterType: "codex_local", + adapterConfig: { model: "gpt-5", env: { API_KEY: { type: "plain", value: "***REDACTED***" } }, extra: { enabled: true } }, + runtimeConfig: { heartbeat: { enabled: false, intervalSec: 3600 }, custom: { nested: [1, "on", false] } }, + defaultEnvironmentId: null, budgetMonthlyCents: 12000, metadata: null, +}; + +describe("agent configuration YAML", () => { + it("round trips the full configuration, multiline text, redactions and adapter extensions", () => { + const text = serializeAgentConfiguration(configuration); + expect(text).toContain("apiVersion: paperclip/v1"); + expect(text).toContain("capabilities: |"); + expect(agentConfigurationPatch(parseAgentConfigurationYaml(text), configuration)).toEqual({}); + }); + + 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; + doc.spec.budgetMonthlyCents = 20000; + expect(agentConfigurationPatch(doc, configuration)).toEqual({ + adapterConfig: { model: "gpt-5", env: configuration.adapterConfig.env }, + replaceAdapterConfig: true, budgetMonthlyCents: 20000, + }); + }); + + it("treats formatting, comments and reordered maps as a no-op", () => { + const text = serializeAgentConfiguration({ ...configuration, adapterConfig: { extra: { enabled: true }, env: configuration.adapterConfig.env, model: "gpt-5" } }); + expect(agentConfigurationPatch(parseAgentConfigurationYaml(`# comment\n${text}`), configuration)).toEqual({}); + }); + + 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`], + ["second document", (text: string) => `${text}---\nkind: Agent\n`], + ["alias", (text: string) => text.replace("metadata: null", "metadata: {a: &a [1], b: *a}")], + ["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`], + ["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 new file mode 100644 index 00000000000..9311a322233 --- /dev/null +++ b/ui/src/lib/agent-configuration-yaml.ts @@ -0,0 +1,68 @@ +import { isAlias, parseDocument, stringify, visit } from "yaml"; +import { + agentConfigurationDocumentSchema, + agentConfigurationSpec, + canonicalAgentConfiguration, + type AgentConfigurationDocument, + type AgentConfigurationSource, +} from "@paperclipai/shared"; + +export function serializeAgentConfiguration(agent: AgentConfigurationSource): string { + return serializeAgentConfigurationDocument({ apiVersion: "paperclip/v1", kind: "Agent", spec: agentConfigurationSpec(agent) } as AgentConfigurationDocument); +} + +export function serializeAgentConfigurationDocument(document: AgentConfigurationDocument): string { + return stringify(document, { + lineWidth: 0, aliasDuplicateObjects: false, + }); +} + +function assertJson(value: unknown, depth = 0): void { + if (depth > 50) throw new Error("Configuration nesting must not exceed 50 levels."); + if (value === null || typeof value === "string" || typeof value === "boolean") return; + if (typeof value === "number" && Number.isFinite(value)) return; + if (Array.isArray(value)) { value.forEach(item => assertJson(item, depth + 1)); return; } + if (value && typeof value === "object") { + for (const [key, item] of Object.entries(value)) { + if (["__proto__", "constructor", "prototype", "<<"].includes(key)) { + throw new Error(`Unsupported configuration key: ${key}`); + } + assertJson(item, depth + 1); + } + return; + } + throw new Error("Configuration must contain only JSON-compatible values (no NaN or infinity)."); +} + +export function parseAgentConfigurationYaml(source: string): AgentConfigurationDocument { + 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]; + if (problem) throw new Error(problem.message); + if (document.directives?.yaml.version !== "1.2") throw new Error("Use YAML 1.2."); + visit(document, (_key, node) => { + if (isAlias(node)) throw new Error("YAML aliases are not supported. Write each value explicitly."); + if (node && typeof node === "object" && "anchor" in node && node.anchor) throw new Error("YAML anchors are not supported."); + if (node && typeof node === "object" && "tag" in node && node.tag) throw new Error("YAML tags are not supported."); + }); + const value: unknown = document.toJS({ maxAliasCount: 0 }); + assertJson(value); + const parsed = agentConfigurationDocumentSchema.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) { + const before = agentConfigurationSpec(saved); + const patch: Record = {}; + for (const key of Object.keys(before) as Array) { + const value = document.spec[key]; + if (canonicalAgentConfiguration(value) !== canonicalAgentConfiguration(before[key as keyof typeof before])) { + patch[key] = value; + } + } + if (Object.hasOwn(patch, "adapterConfig")) patch.replaceAdapterConfig = true; + return patch; +} diff --git a/ui/src/lib/agent-configuration/channels.ts b/ui/src/lib/agent-configuration/channels.ts new file mode 100644 index 00000000000..b599b9a52da --- /dev/null +++ b/ui/src/lib/agent-configuration/channels.ts @@ -0,0 +1,52 @@ +import type { AgentConfigurationDocument } from "@paperclipai/shared"; +import { chatEndpointsApi } from "../../api/chatEndpoints"; +import { equal, type AgentConfigurationSnapshot } from "./load"; +import { requireItem, requireExplicitFields, unique, type AddConfigurationStep } from "./steps"; + +export function planChannelChanges(snapshot: AgentConfigurationSnapshot, document: AgentConfigurationDocument, add: AddConfigurationStep) { + const { id: agentId, companyId } = snapshot.agent; + const before = snapshot.document.spec.channels!; + const next = document.spec.channels; + if (next) { + unique(next.map(row => row.id), "channels"); + for (const row of next) { + const previous = requireItem(before, channel => channel.id === row.id, `Channel ${row.id}`); + if (row.provider !== previous.provider) throw new Error("A channel provider cannot be changed. Create a new channel instead."); + unique(row.resources.map(resource => resource.id), `channels.${row.id}.resources`); + for (const resource of row.resources) { + const existing = requireItem(previous.resources, value => value.id === resource.id, `Channel resource ${resource.id}`); + requireExplicitFields(existing, resource, `channels.${row.id}.resources.${resource.id}`); + } + if (!equal(previous.settings, row.settings)) { + requireExplicitFields(previous.settings, row.settings, `channels.${row.id}.settings`); + const changes = Object.fromEntries(Object.entries(row.settings).filter(([key, value]) => + !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 }); + add(`Channel resources: ${row.id}`, () => chatEndpointsApi.updateResources(row.id, resources)); + } + } + for (const previous of before) if (!next.some(row => row.id === previous.id)) { + add(`Remove channel: ${previous.id}`, () => chatEndpointsApi.setup(previous.id, { action: "remove" })); + } + } + const operations = document.spec.channelOperations; + if (!operations) return; + unique(operations.setup.map(row => row.id), "channelOperations.setup"); + for (const input of operations.create) add(`Create ${input.provider} channel`, () => chatEndpointsApi.create(companyId, { ...input, assignedAgentId: agentId })); + for (const operation of operations.setup) { + const previous = requireItem(before, channel => channel.id === operation.id, `Channel ${operation.id}`); + if (operation.input.action === "remove" && next && !equal(next.find(row => row.id === operation.id), previous)) throw new Error(`Channel ${operation.id} cannot be edited and removed in the same save.`); + if (next && !next.some(row => row.id === operation.id)) throw new Error(`Channel ${operation.id} is both removed and configured.`); + add(`Channel ${operation.input.action}: ${operation.id}`, () => chatEndpointsApi.setup(operation.id, operation.input)); + } + unique(operations.revokeIdentityLinks.map(row => `${row.endpointId}/${row.principalId}`), "channelOperations.revokeIdentityLinks"); + for (const operation of operations.revokeIdentityLinks) { + if ((next && !next.some(row => row.id === operation.endpointId)) || operations.setup.some(row => row.id === operation.endpointId && row.input.action === "remove")) throw new Error(`Channel ${operation.endpointId} cannot be removed and have its identities edited in the same save.`); + const channel = requireItem(snapshot.channels, row => row.detail.id === operation.endpointId, `Channel ${operation.endpointId}`); + requireItem(channel.principals, row => row.principalId === operation.principalId, `Identity ${operation.principalId}`); + add(`Revoke channel identity: ${operation.principalId}`, () => chatEndpointsApi.revokeLink(operation.endpointId, operation.principalId)); + } +} diff --git a/ui/src/lib/agent-configuration/load.ts b/ui/src/lib/agent-configuration/load.ts new file mode 100644 index 00000000000..833ff130eef --- /dev/null +++ b/ui/src/lib/agent-configuration/load.ts @@ -0,0 +1,119 @@ +import { + agentConfigurationSpec, canonicalAgentConfiguration, + type AgentConfigurationDocument, type AgentInstructionsFileDetail, type ToolProfileWithDetails, type ToolPolicy, +} from "@paperclipai/shared"; +import { agentsApi } from "../../api/agents"; +import { toolsApi } from "../../api/tools"; +import { chatEndpointsApi } from "../../api/chatEndpoints"; +import { secretsApi } from "../../api/secrets"; + +export const ownedAdapterKeys = ["paperclipSkillSync", "instructionsBundleMode", "instructionsRootPath", "instructionsEntryFile", "instructionsFilePath"] as const; +export const equal = (a: unknown, b: unknown) => canonicalAgentConfiguration(a) === canonicalAgentConfiguration(b); +export const jsonValue = (value: T): T => JSON.parse(JSON.stringify(value)); + +export async function mapBounded(items: T[], read: (item: T) => Promise): Promise { + const results: R[] = new Array(items.length); + let next = 0; + await Promise.all(Array.from({ length: Math.min(items.length, 4) }, async () => { + while (next < items.length) { const index = next++; results[index] = await read(items[index]); } + })); + return results; +} + +export function profileConfiguration(profile: ToolProfileWithDetails) { + return { + profileKey: profile.profileKey, name: profile.name, description: profile.description, + status: profile.status, defaultAction: profile.defaultAction, metadata: profile.metadata, + entries: profile.entries.map(({ selectorType, effect, applicationId, connectionId, catalogEntryId, toolName, riskLevel, conditions }) => + ({ selectorType, effect, applicationId, connectionId, catalogEntryId, toolName, riskLevel, conditions })), + }; +} + +export function policyConfiguration(policy: ToolPolicy) { + const { name, description, policyType, priority, enabled, selectors, conditions, config } = policy; + return { name, description, policyType, priority, enabled, selectors, conditions, config }; +} + +export function policyAgentIds(policy: Pick): string[] { + return [policy.selectors.agentId, policy.selectors.agentIds].flatMap(value => + Array.isArray(value) ? value.filter((id): id is string => typeof id === "string") : typeof value === "string" ? [value] : []); +} + +export async function loadAgentConfiguration(id: string, companyId: string) { + const core = await agentsApi.getConfiguration(id, companyId); + const [agent, bundle, skills, connections, profiles, policies, endpoints, keys, revisions, secrets, proposals] = await Promise.all([ + agentsApi.get(id, companyId), agentsApi.instructionsBundle(id, companyId), agentsApi.skills(id, companyId), + toolsApi.listConnections(companyId), toolsApi.listProfiles(companyId), toolsApi.listPolicies(companyId), + chatEndpointsApi.list(companyId), agentsApi.listKeys(id, companyId), agentsApi.listConfigRevisions(id, companyId), + secretsApi.list(companyId), secretsApi.listProposals(companyId), + ]); + const [files, channels] = await Promise.all([ + mapBounded(bundle.files.filter(file => file.editable && !file.binary && !file.virtual), file => agentsApi.instructionsFile(id, file.path, companyId)), + mapBounded(endpoints.filter(endpoint => endpoint.assignedAgentId === id && endpoint.status !== "archived"), async endpoint => { + const [detail, resources, principals] = await Promise.all([ + chatEndpointsApi.get(endpoint.id), chatEndpointsApi.listResources(endpoint.id), chatEndpointsApi.listPrincipals(endpoint.id), + ]); + if (detail.assignedAgentId !== id || detail.companyId !== companyId) throw new Error("Channel ownership changed. Reload configuration."); + return { detail, resources, principals }; + }), + ]); + const adapterConfig = { ...core.adapterConfig }; + for (const key of ownedAdapterKeys) delete adapterConfig[key]; + const directPolicies = policies.policies.filter(policy => { + const ids = policyAgentIds(policy); return ids.length > 0 && ids.every(value => value === id); + }); + const directProfiles = profiles.profiles.flatMap(profile => { + const binding = profile.bindings.find(binding => binding.targetType === "agent" && binding.targetId === id); + return binding ? [{ profileId: profile.id, updateSharedDefinition: false, priority: binding.priority, metadata: binding.metadata, configuration: profileConfiguration(profile) }] : []; + }); + const document = jsonValue({ + apiVersion: "paperclip/v1", kind: "Agent", + spec: { + ...agentConfigurationSpec(core), adapterConfig, + permissions: { + canCreateAgents: Boolean(agent.permissions.canCreateAgents), canCreateSkills: agent.permissions.canCreateSkills !== false, + canAssignTasks: Boolean(agent.access?.canAssignTasks), trustPreset: agent.permissions.trustPreset ?? "standard", + ...(agent.permissions.authorizationPolicy ? { authorizationPolicy: agent.permissions.authorizationPolicy } : {}), + }, + 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 })), + tools: { + connections: connections.connections.map(connection => ({ connectionId: connection.id, + installed: (connection.installs ?? []).some(install => install.targetType === "agent" && install.targetId === id) })), + profiles: directProfiles, + policies: directPolicies.map(policy => ({ id: policy.id, configuration: policyConfiguration(policy) })), + }, + channels: channels.map(({ detail, resources }) => ({ id: detail.id, provider: detail.provider, settings: { + executionDefaults: detail.executionDefaults ?? null, + ...(detail.provider === "slack" ? { communicationInstructions: detail.communicationInstructions ?? "" } : {}), + allowUnlinkedPeople: detail.allowUnlinkedPeople, + ...(detail.allowDirectMessages !== undefined ? { allowDirectMessages: detail.allowDirectMessages } : {}), + ...(detail.provider === "slack" && detail.requireAtMention !== undefined ? { requireAtMention: detail.requireAtMention } : {}), + ...(detail.allowGroupChats !== undefined ? { allowGroupChats: detail.allowGroupChats } : {}), + ...(detail.setup?.slackApp ? { slackApp: detail.setup.slackApp } : {}), + ...(detail.setup?.slackSetupMethod ? { slackSetupMethod: detail.setup.slackSetupMethod } : {}), + }, resources: resources.map(resource => ({ id: resource.id, enabled: resource.enabled, executionDefaults: resource.executionDefaults ?? null })) })), + apiKeys: { create: [], revoke: [] }, revisions: { restore: null }, + 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 }; +} + +export type AgentConfigurationSnapshot = Awaited>; diff --git a/ui/src/lib/agent-configuration/save.test.ts b/ui/src/lib/agent-configuration/save.test.ts new file mode 100644 index 00000000000..e497d0b9c0b --- /dev/null +++ b/ui/src/lib/agent-configuration/save.test.ts @@ -0,0 +1,315 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { appearanceForPalette, type AgentConfigurationDocument } from "@paperclipai/shared"; +import { parseAgentConfigurationYaml, serializeAgentConfigurationDocument } from "../agent-configuration-yaml"; +import { loadAgentConfiguration } from "./load"; +import { ConfigurationSaveInterrupted, planAgentConfiguration, saveAgentConfiguration } from "./save"; + +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()])), + tools: Object.fromEntries(["listConnections", "listProfiles", "listPolicies", "getConnectionInstalls", "putConnectionInstalls", "createProfile", "updateProfile", "bindProfile", "unbindProfile", "createPolicy", "updatePolicy", "deletePolicy"].map(key => [key, vi.fn()])), + channels: Object.fromEntries(["list", "get", "listResources", "listPrincipals", "update", "updateResources", "create", "setup", "revokeLink"].map(key => [key, vi.fn()])), + secrets: Object.fromEntries(["list", "listProposals", "create", "rotate", "update", "approveProposal", "rejectProposal"].map(key => [key, vi.fn()])), +})); +vi.mock("../../api/agents", () => ({ agentsApi: api.agents })); +vi.mock("../../api/tools", () => ({ toolsApi: api.tools })); +vi.mock("../../api/chatEndpoints", () => ({ chatEndpointsApi: api.channels })); +vi.mock("../../api/secrets", () => ({ secretsApi: api.secrets })); +const agentId = "11111111-1111-4111-8111-111111111111"; +const resourceId = "22222222-2222-4222-8222-222222222222"; +const agent = { + id: agentId, companyId: "company", name: "Codie", role: "engineer", title: null, icon: null, + appearance: appearanceForPalette("bubblegum-sky"), reportsTo: null, capabilities: null, + adapterType: "process", adapterConfig: { command: "echo", instructionsBundleMode: "managed", paperclipSkillSync: { desiredSkills: ["paperclip"] } }, + runtimeConfig: {}, defaultEnvironmentId: null, budgetMonthlyCents: 0, metadata: null, + permissions: { canCreateAgents: false, canCreateSkills: true }, access: { canAssignTasks: false }, + configurationRevision: "a".repeat(64), +}; +const file = { path: "AGENTS.md", content: "# Codie\n", contentHash: "hash", revision: { id: "revision-file" }, editable: true, binary: false }; +const installs = [{ targetType: "agent", targetId: agentId }, { targetType: "agent", targetId: "another-agent" }]; +const connection = { id: "connection", name: "GitHub", installs }; +const channel = { id: "channel", companyId: "company", assignedAgentId: agentId, provider: "slack", status: "active", allowUnlinkedPeople: false }; +const progress = vi.fn(), receipt = vi.fn(); +const copy = (value: T): T => structuredClone(value); +async function snapshot() { return loadAgentConfiguration(agentId, "company"); } +function roundTrip(document: AgentConfigurationDocument) { return parseAgentConfigurationYaml(serializeAgentConfigurationDocument(document)); } + +beforeEach(() => { + vi.resetAllMocks(); + api.agents.getConfiguration.mockResolvedValue(copy(agent)); api.agents.get.mockResolvedValue(copy(agent)); + api.agents.instructionsBundle.mockResolvedValue({ mode: "managed", rootPath: "/managed/codie", entryFile: "AGENTS.md", editable: true, files: [file], warnings: [] }); + api.agents.instructionsFile.mockResolvedValue(file); + 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.listProfiles.mockResolvedValue({ profiles: [] }); api.tools.listPolicies.mockResolvedValue({ policies: [] }); + api.tools.getConnectionInstalls.mockResolvedValue({ installs }); + api.tools.createProfile.mockResolvedValue({ id: "new-profile" }); + api.channels.list.mockResolvedValue([channel]); api.channels.get.mockResolvedValue(channel); + api.channels.listResources.mockResolvedValue([{ id: resourceId, enabled: true }]); api.channels.listPrincipals.mockResolvedValue([{ principalId: "principal" }]); + api.secrets.list.mockResolvedValue([{ id: "secret", name: "Token", provider: "local_encrypted", latestVersion: 1 }]); + api.secrets.listProposals.mockResolvedValue([{ id: "proposal", target: { id: agentId }, proposedBy: { id: agentId } }]); + api.agents.createKey.mockResolvedValue({ id: "new-key", name: "Automation", token: "one-time-token" }); + api.secrets.create.mockResolvedValue({ id: "new-secret", name: "Token" }); +}); + +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.skills).toEqual([{ key: "paperclip", versionId: resourceId }]); + expect(doc.spec.adapterConfig).toEqual({ command: "echo" }); + expect(doc.observed?.secretProposals).toHaveLength(1); + expect(planAgentConfiguration(saved, doc)).toEqual([]); + await saveAgentConfiguration(saved, doc, receipt, progress); + expect(api.agents.getConfiguration).toHaveBeenCalledTimes(1); + expect(api.agents.update).not.toHaveBeenCalled(); + }); + + it("round trips existing profiles and agent policies without rewriting shared definitions", async () => { + api.tools.listProfiles.mockResolvedValue({ profiles: [{ id: "profile", profileKey: "read", name: "Read", description: null, + status: "active", defaultAction: "deny", metadata: null, + bindings: [{ targetType: "agent", targetId: agentId, priority: 100, metadata: null }], + entries: [{ selectorType: "tool_name", effect: "include", applicationId: null, connectionId: null, + catalogEntryId: null, toolName: "get_repository", riskLevel: null, conditions: null }], + }] }); + api.tools.listPolicies.mockResolvedValue({ policies: [{ id: "policy", name: "Read", description: null, policyType: "allow", + priority: 100, enabled: true, selectors: { agentIds: [agentId] }, conditions: null, config: {} }] }); + const saved = await snapshot(); + expect(planAgentConfiguration(saved, roundTrip(saved.document))).toEqual([]); + const doc = copy(saved.document); doc.spec.tools!.profiles = []; doc.spec.tools!.policies = []; + await saveAgentConfiguration(saved, roundTrip(doc), receipt, progress); + expect(api.tools.unbindProfile).toHaveBeenCalledWith("company", "profile", { targetType: "agent", targetId: agentId }); + expect(api.tools.deletePolicy).toHaveBeenCalledWith("company", "policy"); + expect(api.tools.updateProfile).not.toHaveBeenCalled(); + }); + + it("switches instruction locations without overwriting files at the destination", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.instructions!.rootPath = "/workspace/new"; doc.spec.instructions!.mode = "external"; + api.agents.updateInstructionsBundle.mockResolvedValue({ files: [file] }); + api.agents.instructionsFile.mockResolvedValueOnce(file).mockResolvedValueOnce({ ...file, content: "Destination instructions" }); + const result = await saveAgentConfiguration(saved, roundTrip(doc), receipt, progress); + expect(api.agents.updateInstructionsBundle).toHaveBeenCalledWith(agentId, { mode: "external", rootPath: "/workspace/new", entryFile: "AGENTS.md" }, "company"); + expect(api.agents.saveInstructionsFile).not.toHaveBeenCalled(); + expect(api.agents.deleteInstructionsFile).not.toHaveBeenCalled(); + expect(result.document.spec.instructions!.files["AGENTS.md"]).toBe("Destination instructions"); + }); + + it("rejects a location change combined with file edits before applying an unrelated core edit", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.title = "Engineer"; doc.spec.instructions!.entryFile = "NEW.md"; + doc.spec.instructions!.files["NEW.md"] = "New instructions"; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("location or entry file first"); + expect(api.agents.update).not.toHaveBeenCalled(); + expect(api.agents.updateInstructionsBundle).not.toHaveBeenCalled(); + }); + + it("rejects ignored managed-root changes and impossible task-assignment permissions", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.instructions!.rootPath = "/ignored/root"; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("assigned by Paperclip"); + doc.spec.instructions = saved.document.spec.instructions; + doc.spec.permissions!.canCreateAgents = true; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("task assignment authority"); + expect(api.agents.updatePermissions).not.toHaveBeenCalled(); + }); + + it("creates a secret and binds its returned ID without exporting the value", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.secrets!.create = [{ secret: { name: "Token", value: "write-only-value", provider: "local_encrypted" }, bind: { env: ["TOKEN"], api: ["SERVICE"] } }]; + const result = await saveAgentConfiguration(saved, roundTrip(doc), receipt, progress); + expect(api.secrets.create).toHaveBeenCalledWith("company", expect.objectContaining({ value: "write-only-value" })); + expect(api.agents.update).toHaveBeenCalledWith(agentId, expect.objectContaining({ + adapterConfig: expect.objectContaining({ env: { TOKEN: { type: "secret_ref", secretId: "new-secret", version: "latest" } }, + "access.SERVICE": { type: "secret_ref", secretId: "new-secret", version: "latest" } }), + expectedConfigurationRevision: agent.configurationRevision, + }), "company"); + expect(JSON.stringify(result.document)).not.toContain("write-only-value"); + }); + + it("rejects nested schema typos and unsupported channel creation before writes", async () => { + const saved = await snapshot(); + const doc = copy(saved.document); + Object.assign(doc.spec.skills![0] as object, { typo: true }); + expect(() => roundTrip(doc)).toThrow("spec.skills.0"); + doc.spec.skills = saved.document.spec.skills; + doc.spec.channelOperations!.create = [{ provider: "telegram", slackSetupMethod: "manual" }]; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("Slack app details"); + expect(api.channels.create).not.toHaveBeenCalled(); + }); + + it("saves each domain through its owning API and preserves other agents' installations", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.title = "Engineer"; + doc.spec.instructions!.files["AGENTS.md"] = "# Revised\n"; + doc.spec.skills = []; + doc.spec.permissions!.canCreateSkills = false; + doc.spec.tools!.connections[0].installed = false; + doc.spec.channels![0].settings.allowUnlinkedPeople = true; + doc.spec.channels![0].resources = []; + doc.spec.apiKeys = { create: [{ name: "Automation", scope: { kind: "standard" } }], revoke: ["key"] }; + doc.spec.channelOperations!.revokeIdentityLinks = [{ endpointId: "channel", principalId: "principal" }]; + doc.spec.secrets!.rotate = [{ id: "secret", input: { value: "replacement" } }]; + doc.spec.secrets!.proposals = [{ id: "proposal", decision: "reject", reason: "Wrong account" }]; + await saveAgentConfiguration(saved, roundTrip(doc), receipt, progress); + expect(api.agents.update).toHaveBeenCalledExactlyOnceWith(agentId, { title: "Engineer", expectedConfigurationRevision: agent.configurationRevision }, "company"); + expect(api.agents.saveInstructionsFile).toHaveBeenCalledWith(agentId, { path: "AGENTS.md", content: "# Revised\n", baseHash: "hash", baseRevisionId: "revision-file" }, "company"); + expect(api.agents.syncSkills).toHaveBeenCalledWith(agentId, [], "replace", "company"); + expect(api.tools.putConnectionInstalls).toHaveBeenCalledWith("connection", [{ targetType: "agent", targetId: "another-agent" }]); + expect(api.channels.update).toHaveBeenCalledExactlyOnceWith("channel", { allowUnlinkedPeople: true }); + expect(api.channels.updateResources).toHaveBeenCalledWith("channel", [{ id: resourceId, enabled: false, executionDefaults: null }]); + expect(api.channels.revokeLink).toHaveBeenCalledWith("channel", "principal"); + expect(api.agents.updatePermissions).toHaveBeenCalledWith(agentId, expect.objectContaining({ canCreateSkills: false }), "company"); + expect(api.secrets.rotate).toHaveBeenCalledWith("secret", { value: "replacement" }); + expect(api.secrets.rejectProposal).toHaveBeenCalledWith("company", "proposal", { reason: "Wrong account" }); + expect(api.agents.revokeKey).toHaveBeenCalledWith(agentId, "key", "company"); + expect(receipt).toHaveBeenCalledWith({ label: "API key: Automation", id: "new-key", token: "one-time-token" }); + }); + + it("requires explicit clears for nested PATCH fields", 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(); + }); + + it("requires explicit consent for shared profile changes and still supports binding without changing them", async () => { + const profile = { id: "profile", profileKey: "shared", name: "Shared", description: null, + status: "active", defaultAction: "deny", metadata: null, entries: [], + bindings: [{ targetType: "agent", targetId: "another-agent", priority: 100, metadata: null }] }; + api.tools.listProfiles.mockResolvedValue({ profiles: [profile] }); + const saved = await snapshot(); const doc = copy(saved.document); + const configuration = { profileKey: "shared", name: "Shared", description: null, status: "active" as const, defaultAction: "deny" as const, metadata: null, entries: [] }; + doc.spec.tools!.profiles = [{ profileId: "profile", priority: 100, metadata: null, configuration }]; + await saveAgentConfiguration(saved, doc, receipt, progress); + expect(api.tools.bindProfile).toHaveBeenCalledTimes(1); + expect(api.tools.updateProfile).not.toHaveBeenCalled(); + configuration.name = "Changed for everyone"; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("updateSharedDefinition: true"); + doc.spec.tools!.profiles[0].updateSharedDefinition = true; + await saveAgentConfiguration(saved, doc, receipt, progress); + expect(api.tools.updateProfile).toHaveBeenCalledWith("profile", configuration); + delete (configuration as Partial).description; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow(".description explicit"); + }); + + it("rejects a newly selected profile whose definition changed after the editor loaded", async () => { + const profile = { id: "profile", profileKey: "unbound", name: "Unbound", description: null, + status: "active", defaultAction: "deny", metadata: null, entries: [], bindings: [] }; + api.tools.listProfiles.mockResolvedValueOnce({ profiles: [profile] }); + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.tools!.profiles = [{ profileId: "profile", priority: 100, metadata: null, configuration: + { profileKey: "unbound", name: "Unbound", description: null, status: "active", defaultAction: "deny", metadata: null, entries: [] } }]; + api.tools.listProfiles.mockResolvedValue({ profiles: [{ ...profile, name: "Changed elsewhere" }] }); + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("Tool profile profile changed"); + expect(api.tools.updateProfile).not.toHaveBeenCalled(); + expect(api.tools.bindProfile).not.toHaveBeenCalled(); + }); + + it("rejects removing a channel while editing its settings or identity links", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.channels![0].settings.communicationInstructions = "New instructions"; + doc.spec.channelOperations!.setup = [{ id: "channel", input: { action: "remove" } }]; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("edited and removed"); + doc.spec.channels = []; doc.spec.channelOperations!.setup = []; + doc.spec.channelOperations!.revokeIdentityLinks = [{ endpointId: "channel", principalId: "principal" }]; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("removed and have its identities edited"); + expect(api.channels.setup).not.toHaveBeenCalled(); + expect(api.channels.update).not.toHaveBeenCalled(); + }); + + it("rejects overlapping secret bindings and preserves a concurrently edited binding", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + const create = { secret: { name: "Token", value: "new-value", provider: "local_encrypted" as const }, bind: { env: ["TOKEN"], api: [] } }; + doc.spec.secrets!.create = [create, { ...create, secret: { ...create.secret, name: "Other" } }]; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("bind.env contains duplicate"); + doc.spec.secrets!.create = [create]; doc.spec.adapterConfig.env = { TOKEN: "competing-value" }; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("not both"); + delete doc.spec.adapterConfig.env; + api.agents.getConfiguration.mockResolvedValueOnce(agent).mockResolvedValueOnce({ ...agent, adapterConfig: { ...agent.adapterConfig, env: { TOKEN: "someone-else" } } }); + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("TOKEN changed"); + expect(api.secrets.create).not.toHaveBeenCalled(); + expect(api.agents.update).not.toHaveBeenCalled(); + }); + + it("retains normalization warnings returned by writes after refreshing the saved snapshot", async () => { + const saved = await snapshot(); const doc = copy(saved.document); doc.spec.title = "Engineer"; + const warning = { code: "runner_option_ignored", field: "chrome", message: "Chrome is ignored" }; + api.agents.update.mockResolvedValue({ ...agent, configurationWarnings: [warning] }); + const result = await saveAgentConfiguration(saved, doc, receipt, progress); + expect(result.agent.configurationWarnings).toEqual([warning]); + }); + + it("checks the entire plan before making any writes", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.title = "Engineer"; doc.spec.apiKeys!.revoke = ["foreign-key"]; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("foreign-key"); + expect(api.agents.update).not.toHaveBeenCalled(); expect(api.agents.getConfiguration).toHaveBeenCalledTimes(1); + }); + + it("rejects stale core, file content and edited channel settings before writes", async () => { + const saved = await snapshot(); const doc = copy(saved.document); doc.spec.title = "Engineer"; + api.agents.getConfiguration.mockResolvedValueOnce({ ...agent, configurationRevision: "b".repeat(64) }); + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("configuration changed"); + doc.spec.instructions!.files["AGENTS.md"] = "My change"; + api.agents.instructionsFile.mockResolvedValueOnce({ ...file, content: "Someone else's change" }); + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("instructions changed"); + delete doc.spec.instructions; + doc.spec.channels![0].settings.communicationInstructions = "Be concise"; + api.channels.get.mockResolvedValueOnce({ ...channel, allowUnlinkedPeople: true }); + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("channels changed"); + expect(api.agents.update).not.toHaveBeenCalled(); + }); + + it("does not undo a concurrent change to an unedited resource section", async () => { + const saved = await snapshot(); const doc = copy(saved.document); doc.spec.title = "Engineer"; + api.channels.get.mockResolvedValue({ ...channel, allowUnlinkedPeople: true }); + await saveAgentConfiguration(saved, doc, receipt, progress); + expect(api.agents.update).toHaveBeenCalledTimes(1); expect(api.channels.update).not.toHaveBeenCalled(); + }); + + it("reports partial progress without retrying or losing a newly created key", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.title = "Engineer"; + doc.spec.apiKeys!.create = [{ name: "Automation", scope: { kind: "standard" } }, { name: "Second", scope: { kind: "standard" } }]; + api.agents.createKey.mockResolvedValueOnce({ id: "new-key", name: "Automation", token: "one-time-token" }).mockRejectedValueOnce(new Error("Network lost")); + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow(ConfigurationSaveInterrupted); + expect(receipt).toHaveBeenCalledWith(expect.objectContaining({ token: "one-time-token" })); + expect(api.agents.update).toHaveBeenCalledTimes(1); expect(api.agents.createKey).toHaveBeenCalledTimes(2); + }); + + it("requires revision restore to be the only change and never replays history", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.revisions!.restore = "revision"; doc.spec.title = "Engineer"; + await expect(saveAgentConfiguration(saved, doc, receipt, progress)).rejects.toThrow("own save"); + doc.spec.title = null; + await saveAgentConfiguration(saved, doc, receipt, progress); + expect(api.agents.rollbackConfigRevision).toHaveBeenCalledWith(agentId, "revision", "company"); + expect(api.agents.update).not.toHaveBeenCalled(); + }); + + 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 = []; + 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"]; + doc.spec.tools!.connections[0].installed = false; + expect(() => planAgentConfiguration(saved, doc)).toThrow("organization-wide"); + }); + + it("creates profiles, scoped policies and channel setup using native contracts", async () => { + const saved = await snapshot(); const doc = copy(saved.document); + doc.spec.tools!.profiles = [{ profileId: null, priority: 10, metadata: null, configuration: { name: "Read", profileKey: "read", status: "active", defaultAction: "deny", entries: [] } }]; + doc.spec.tools!.policies = [{ id: null, configuration: { name: "Safe", policyType: "allow", priority: 10, enabled: true, selectors: {}, conditions: null, config: {} } }]; + doc.spec.channelOperations!.setup = [{ id: "channel", input: { action: "pause" } }]; + await saveAgentConfiguration(saved, roundTrip(doc), receipt, progress); + expect(api.tools.createProfile).toHaveBeenCalledWith("company", expect.objectContaining({ profileKey: "read" })); + expect(api.tools.bindProfile).toHaveBeenCalledWith("company", "new-profile", { targetType: "agent", targetId: agentId, priority: 10, metadata: null }); + expect(api.tools.createPolicy).toHaveBeenCalledWith("company", expect.objectContaining({ selectors: { agentIds: [agentId] } })); + expect(api.channels.setup).toHaveBeenCalledWith("channel", { action: "pause" }); + }); +}); diff --git a/ui/src/lib/agent-configuration/save.ts b/ui/src/lib/agent-configuration/save.ts new file mode 100644 index 00000000000..f60bce0490b --- /dev/null +++ b/ui/src/lib/agent-configuration/save.ts @@ -0,0 +1,208 @@ +import { agentConfigurationDocumentSchema, type AgentConfigurationDocument, type AgentConfigurationWarning } from "@paperclipai/shared"; +import { agentsApi } from "../../api/agents"; +import { secretsApi } from "../../api/secrets"; +import { agentConfigurationPatch } from "../agent-configuration-yaml"; +import { equal, loadAgentConfiguration, ownedAdapterKeys, type AgentConfigurationSnapshot } from "./load"; +import { planToolChanges } from "./tools"; +import { planChannelChanges } from "./channels"; +import { requireItem, unique, type ConfigurationStep, type AddConfigurationStep } from "./steps"; + +export interface ConfigurationReceipt { label: string; id: string; token?: string } +export class ConfigurationSaveInterrupted extends Error { + constructor(label: string, completed: string[], cause: unknown) { + super(`${completed.length ? `Saved: ${completed.join(", ")}. ` : ""}Could not complete ${label}: ${cause instanceof Error ? cause.message : "Request failed"}. Some changes may have been saved. Reload saved before saving again.`); + } +} + +function planInstructions(snapshot: AgentConfigurationSnapshot, document: AgentConfigurationDocument, add: AddConfigurationStep) { + const next = document.spec.instructions; + if (!next) return; + const previous = snapshot.document.spec.instructions!; + if (equal(previous, next)) return; + if (!snapshot.bundle.editable && !next.mode) throw new Error("Choose managed or external instructions mode before adding files."); + if (next.mode === null && previous.mode !== null) throw new Error("Choose managed or external instructions mode."); + if (next.mode === "external" && !next.rootPath?.startsWith("/")) throw new Error("External instructions require an absolute rootPath."); + if (Object.hasOwn(previous.files, next.entryFile) && !Object.hasOwn(next.files, next.entryFile)) throw new Error("The instructions entry file cannot be removed."); + for (const path of Object.keys(next.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.`); + } + const bundleChanged = previous.mode !== next.mode || previous.rootPath !== next.rootPath || previous.entryFile !== next.entryFile; + if (bundleChanged) { + if (!equal(previous.files, next.files)) throw new Error("Save the instructions location or entry file first, then edit the files loaded from that location."); + if (next.mode === "managed" && previous.mode === "managed" && next.rootPath !== previous.rootPath) { + throw new Error("The managed instructions root is assigned by Paperclip. Choose external mode to use another folder."); + } + add("Instructions location (load the destination files)", () => agentsApi.updateInstructionsBundle(snapshot.agent.id, { + ...(next.mode ? { mode: next.mode } : {}), rootPath: next.rootPath, entryFile: next.entryFile, + }, snapshot.agent.companyId)); + return; + } + const currentFiles = snapshot.files; + for (const [path, content] of Object.entries(next.files)) { + if (previous.files[path] === content) continue; + add(`Instructions: ${path}`, () => { + const file = currentFiles.get(path); + if (file?.content === content) return Promise.resolve(); + return agentsApi.saveInstructionsFile(snapshot.agent.id, { + path, content, baseHash: file?.contentHash ?? null, baseRevisionId: file?.revision?.id ?? null, + }, snapshot.agent.companyId); + }); + } + for (const path of Object.keys(previous.files)) { + if (Object.hasOwn(next.files, path)) continue; + add(`Remove instruction file: ${path}`, async () => { + const file = currentFiles.get(path); + if (!file) return; + return agentsApi.deleteInstructionsFile(snapshot.agent.id, path, snapshot.agent.companyId, file.contentHash); + }); + } +} + +function planSecrets(snapshot: AgentConfigurationSnapshot, document: AgentConfigurationDocument, add: AddConfigurationStep, receipt: (value: ConfigurationReceipt) => void) { + const next = document.spec.secrets; + if (!next) return; + const { id, companyId } = snapshot.agent; + unique(next.rotate.map(row => row.id), "secrets.rotate"); + unique(next.update.map(row => row.id), "secrets.update"); + unique(next.proposals.map(row => row.id), "secrets.proposals"); + unique(next.create.map(row => row.secret.key ?? row.secret.name), "secrets.create"); + for (const operation of [...next.rotate, ...next.update]) requireItem(snapshot.secrets, secret => secret.id === operation.id, `Secret ${operation.id}`); + unique(next.create.flatMap(operation => operation.bind.env), "secrets.create.bind.env"); + unique(next.create.flatMap(operation => operation.bind.api), "secrets.create.bind.api"); + const env = (config: Record) => config.env && typeof config.env === "object" && !Array.isArray(config.env) ? config.env as Record : {}; + for (const operation of next.create) { + for (const key of operation.bind.env) { + if (!equal(env(document.spec.adapterConfig)[key], env(snapshot.document.spec.adapterConfig)[key])) throw new Error(`Set env.${key} either in adapterConfig or secrets.create.bind, not both in one save.`); + } + for (const alias of operation.bind.api) { + if (!equal(document.spec.adapterConfig[`access.${alias}`], snapshot.document.spec.adapterConfig[`access.${alias}`])) throw new Error(`Set access.${alias} either in adapterConfig or secrets.create.bind, not both in one save.`); + } + const assertBindingsCurrent = (config: Record) => { + for (const key of operation.bind.env) if (!equal(env(config)[key], env(snapshot.document.spec.adapterConfig)[key])) throw new Error(`env.${key} changed before secret binding. Reload saved before saving.`); + for (const alias of operation.bind.api) if (!equal(config[`access.${alias}`], snapshot.document.spec.adapterConfig[`access.${alias}`])) throw new Error(`access.${alias} changed before secret binding. Reload saved before saving.`); + }; + add(`Create secret: ${operation.secret.name}`, async () => { + if (operation.bind.env.length || operation.bind.api.length) assertBindingsCurrent((await agentsApi.getConfiguration(id, companyId)).adapterConfig); + const created = await secretsApi.create(companyId, operation.secret); + receipt({ label: `Secret: ${created.name}`, id: created.id }); + if (!operation.bind.env.length && !operation.bind.api.length) return; + const current = await agentsApi.getConfiguration(id, companyId); + assertBindingsCurrent(current.adapterConfig); + const adapterConfig = { ...current.adapterConfig }; + const binding = { type: "secret_ref", secretId: created.id, version: "latest" }; + if (operation.bind.env.length) adapterConfig.env = { ...(adapterConfig.env as Record ?? {}), + ...Object.fromEntries(operation.bind.env.map(key => [key, binding])) }; + for (const alias of operation.bind.api) adapterConfig[`access.${alias}`] = binding; + await agentsApi.update(id, { adapterConfig, replaceAdapterConfig: true, expectedConfigurationRevision: current.configurationRevision }, companyId); + }); + } + for (const operation of next.rotate) add(`Rotate secret: ${operation.id}`, () => secretsApi.rotate(operation.id, operation.input)); + for (const operation of next.update) add(`Update secret: ${operation.id}`, () => secretsApi.update(operation.id, operation.input)); + for (const operation of next.proposals) { + requireItem(snapshot.proposals, proposal => proposal.id === operation.id && (proposal.target?.id === id || proposal.proposedBy.id === id), `Secret proposal ${operation.id}`); + if (operation.decision === "reject" && !operation.reason?.trim()) throw new Error("A rejected secret proposal requires a reason."); + add(`Secret proposal: ${operation.id}`, () => operation.decision === "approve" + ? secretsApi.approveProposal(companyId, operation.id) + : secretsApi.rejectProposal(companyId, operation.id, { reason: operation.reason! })); + } +} + +export function planAgentConfiguration(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."); + for (const key of ownedAdapterKeys) if (Object.hasOwn(document.spec.adapterConfig, key)) throw new Error(`Use the ${key === "paperclipSkillSync" ? "skills" : "instructions"} section instead of adapterConfig.${key}.`); + const adapterConfig = { ...document.spec.adapterConfig }; + for (const key of ownedAdapterKeys) if (Object.hasOwn(snapshot.core.adapterConfig, key)) adapterConfig[key] = snapshot.core.adapterConfig[key]; + const corePatch = agentConfigurationPatch({ ...document, spec: { ...document.spec, adapterConfig } }, snapshot.core); + if (Object.keys(corePatch).length) add("Agent configuration", () => agentsApi.update(snapshot.agent.id, { + ...corePatch, expectedConfigurationRevision: snapshot.core.configurationRevision, + }, snapshot.agent.companyId)); + if (document.spec.permissions && !document.spec.permissions.canAssignTasks && (document.spec.role === "ceo" || document.spec.permissions.canCreateAgents)) { + throw new Error("CEO agents and agents that can create agents also have task assignment authority. Set canAssignTasks to true."); + } + planInstructions(snapshot, document, add); + if (document.spec.skills && !equal(document.spec.skills, snapshot.document.spec.skills)) { + unique(document.spec.skills.map(skill => typeof skill === "string" ? skill : skill.key), "skills"); + add("Skills and version pins", () => agentsApi.syncSkills(snapshot.agent.id, document.spec.skills!, "replace", snapshot.agent.companyId)); + } + planToolChanges(snapshot, document, add); + planChannelChanges(snapshot, document, add); + planSecrets(snapshot, document, add, receipt); + if (document.spec.permissions && !equal(document.spec.permissions, snapshot.document.spec.permissions)) { + add("Permissions and trust", () => agentsApi.updatePermissions(snapshot.agent.id, { ...document.spec.permissions!, + authorizationPolicy: document.spec.permissions!.authorizationPolicy ?? {}, + }, snapshot.agent.companyId)); + } + const keys = document.spec.apiKeys; + if (keys) { + unique(keys.revoke, "apiKeys.revoke"); + unique(keys.create.map(key => key.name), "apiKeys.create"); + for (const id of keys.revoke) { + const key = requireItem(snapshot.keys, key => key.id === id, `API key ${id}`); + if (!key.revokedAt) add(`Revoke API key: ${key.name}`, () => agentsApi.revokeKey(snapshot.agent.id, id, snapshot.agent.companyId)); + } + for (const input of keys.create) add(`Create API key: ${input.name}`, async () => { + const key = await agentsApi.createKey(snapshot.agent.id, input.name, snapshot.agent.companyId, input.scope); + receipt({ label: `API key: ${key.name}`, id: key.id, token: key.token }); + }); + } + if (document.spec.revisions?.restore) { + const id = document.spec.revisions.restore; + requireItem(snapshot.revisions, revision => revision.id === id, `Revision ${id}`); + if (steps.length) throw new Error("Restore a revision in its own save, then edit the restored configuration."); + add("Restore configuration revision", () => agentsApi.rollbackConfigRevision(snapshot.agent.id, id, snapshot.agent.companyId)); + } + return steps; +} + +export async function saveAgentConfiguration(snapshot: AgentConfigurationSnapshot, input: AgentConfigurationDocument, + onReceipt: (receipt: ConfigurationReceipt) => void, onProgress: (label: string) => void) { + const document = agentConfigurationDocumentSchema.parse(input); + const initialPlan = planAgentConfiguration(snapshot, document); + 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."); + for (const key of ["permissions", "instructions", "skills", "tools", "channels"] as const) { + if (document.spec[key] !== undefined && !equal(document.spec[key], snapshot.document.spec[key]) && !equal(current.document.spec[key], snapshot.document.spec[key])) { + throw new Error(`${key} changed while you were editing. Reload saved before saving.`); + } + } + const spec = { ...document.spec }; + for (const key of ["permissions", "instructions", "skills", "tools", "channels"] as const) { + if (equal(document.spec[key], snapshot.document.spec[key])) Object.assign(spec, { [key]: current.document.spec[key] }); + } + for (const operation of [...(document.spec.secrets?.rotate ?? []), ...(document.spec.secrets?.update ?? [])]) { + const previous = snapshot.secrets.find(secret => secret.id === operation.id); + if (!equal(previous, current.secrets.find(secret => secret.id === operation.id))) throw new Error(`Secret ${operation.id} changed. Reload saved before saving.`); + } + for (const profile of document.spec.tools?.profiles ?? []) { + if (!profile.profileId) continue; + const originalRow = snapshot.document.spec.tools?.profiles.find(value => value.profileId === profile.profileId); + if (originalRow && equal(originalRow, profile)) continue; + const before = snapshot.profiles.find(value => value.id === profile.profileId); + 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 completed: string[] = []; + const warnings: AgentConfigurationWarning[] = []; + for (const step of steps) { + onProgress(step.label); + try { + const result = await step.apply(); + if (result && typeof result === "object" && "configurationWarnings" in result && Array.isArray(result.configurationWarnings)) { + warnings.push(...result.configurationWarnings as AgentConfigurationWarning[]); + } + completed.push(step.label); + } + catch (cause) { throw new ConfigurationSaveInterrupted(step.label, completed, cause); } + } + try { + const refreshed = await loadAgentConfiguration(snapshot.agent.id, snapshot.agent.companyId); + if (warnings.length) refreshed.agent = { ...refreshed.agent, configurationWarnings: warnings }; + return refreshed; + } + catch (cause) { throw new ConfigurationSaveInterrupted("refreshing the saved configuration", completed, cause); } +} diff --git a/ui/src/lib/agent-configuration/steps.ts b/ui/src/lib/agent-configuration/steps.ts new file mode 100644 index 00000000000..cd4d016922b --- /dev/null +++ b/ui/src/lib/agent-configuration/steps.ts @@ -0,0 +1,18 @@ +export interface ConfigurationStep { label: string; apply: () => Promise } +export type AddConfigurationStep = (label: string, apply: ConfigurationStep["apply"]) => void; + +export function unique(values: string[], field: string) { + if (new Set(values).size !== values.length) throw new Error(`${field} contains duplicate entries.`); +} + +export function requireItem(items: readonly T[], matches: (item: T) => boolean, label: string): T { + const item = items.find(matches); + if (!item) throw new Error(`${label} is not in this agent’s saved configuration. Reload before editing it.`); + return item; +} + +export function requireExplicitFields(before: object, after: object, path: string) { + for (const [key, value] of Object.entries(before)) { + if (value !== undefined && !Object.hasOwn(after, key)) throw new Error(`Keep ${path}.${key} explicit; use null, false, an empty string, or an empty list to clear it where supported.`); + } +} diff --git a/ui/src/lib/agent-configuration/tools.ts b/ui/src/lib/agent-configuration/tools.ts new file mode 100644 index 00000000000..0e12102e360 --- /dev/null +++ b/ui/src/lib/agent-configuration/tools.ts @@ -0,0 +1,70 @@ +import type { AgentConfigurationDocument } from "@paperclipai/shared"; +import { toolsApi } from "../../api/tools"; +import { equal, policyAgentIds, profileConfiguration, type AgentConfigurationSnapshot } from "./load"; +import { requireItem, requireExplicitFields, unique, type AddConfigurationStep } from "./steps"; + +export function planToolChanges(snapshot: AgentConfigurationSnapshot, document: AgentConfigurationDocument, add: AddConfigurationStep) { + const next = document.spec.tools; + if (!next) return; + const before = snapshot.document.spec.tools!; + const { id: agentId, companyId } = snapshot.agent; + unique(next.connections.map(row => row.connectionId), "tools.connections"); + 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 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)!; + if (connection.installs?.some(row => row.targetType === "company")) throw new Error(`${connection.name} is installed organization-wide. Its inherited installation cannot be overridden for one agent.`); + add(`Tool installation: ${connection.name}`, async () => { + const current = await toolsApi.getConnectionInstalls(connection.id); + if (current.installs.some(row => row.targetType === "company") || current.installs.some(row => row.targetType === "agent" && row.targetId === agentId) !== previous.installed) { + throw new Error(`Installation for ${connection.name} changed. Reload before saving.`); + } + const installs = current.installs.filter(row => !(row.targetType === "agent" && row.targetId === agentId)) + .map(({ targetType, targetId }) => ({ targetType, targetId })); + if (installed) installs.push({ targetType: "agent", targetId: agentId }); + return toolsApi.putConnectionInstalls(connection.id, installs); + }); + } + for (const row of next.profiles) { + const existing = row.profileId ? requireItem(snapshot.profiles, profile => profile.id === row.profileId, `Profile ${row.profileId}`) : null; + const previous = before.profiles.find(profile => profile.profileId === row.profileId); + if (previous && equal({ ...previous, updateSharedDefinition: false }, { ...row, updateSharedDefinition: false })) continue; + if (existing) { + requireExplicitFields(profileConfiguration(existing), row.configuration, `tools.profiles.${row.profileId}.configuration`); + const shared = existing.bindings.some(binding => binding.targetType !== "agent" || binding.targetId !== agentId); + if (shared && !equal(profileConfiguration(existing), row.configuration) && !row.updateSharedDefinition) { + throw new Error(`Profile ${existing.name} is shared with other targets. Set updateSharedDefinition: true to change its shared definition, or create a separate profile with profileId: null.`); + } + } + add(`Tool profile${row.updateSharedDefinition ? " (shared definition)" : ""}: ${row.configuration.name}`, async () => { + let profileId = row.profileId; + if (!profileId) profileId = (await toolsApi.createProfile(companyId, row.configuration)).id; + else if (existing && !equal(profileConfiguration(existing), row.configuration)) await toolsApi.updateProfile(profileId, row.configuration); + return toolsApi.bindProfile(companyId, profileId, { targetType: "agent", targetId: agentId, priority: row.priority, metadata: row.metadata }); + }); + } + for (const previous of before.profiles) { + if (!next.profiles.some(row => row.profileId === previous.profileId)) add(`Unbind tool profile: ${previous.configuration.name}`, + () => toolsApi.unbindProfile(companyId, previous.profileId!, { targetType: "agent", targetId: agentId })); + } + for (const row of next.policies) { + const previous = row.id ? requireItem(before.policies, policy => policy.id === row.id, `Agent policy ${row.id}`) : null; + if (previous) requireExplicitFields(previous.configuration, row.configuration, `tools.policies.${row.id}.configuration`); + const agents = policyAgentIds(row.configuration); + if (agents.some(id => id !== agentId)) throw new Error("tools.policies must be scoped only to this agent."); + const selectors: Record = { ...row.configuration.selectors, agentIds: [agentId] }; + delete selectors.agentId; + const configuration = { ...row.configuration, selectors }; + if (previous && equal(previous.configuration, row.configuration)) continue; + add(`Tool policy: ${configuration.name}`, () => row.id + ? toolsApi.updatePolicy(companyId, row.id, configuration) + : toolsApi.createPolicy(companyId, configuration)); + } + for (const previous of before.policies) { + if (!next.policies.some(row => row.id === previous.id)) add(`Remove tool policy: ${previous.configuration.name}`, + () => toolsApi.deletePolicy(companyId, previous.id!)); + } +} diff --git a/ui/src/pages/AgentAdvancedTab.test.tsx b/ui/src/pages/AgentAdvancedTab.test.tsx new file mode 100644 index 00000000000..f2a81500f97 --- /dev/null +++ b/ui/src/pages/AgentAdvancedTab.test.tsx @@ -0,0 +1,153 @@ +// @vitest-environment jsdom +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { EditorView } from "@codemirror/view"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { appearanceForPalette, agentConfigurationSpec, type Agent } from "@paperclipai/shared"; +import { ConfigurationSaveInterrupted } from "../lib/agent-configuration/save"; +import { AgentAdvancedTab } from "./AgentAdvancedTab"; + +const api = vi.hoisted(() => ({ load: vi.fn(), save: vi.fn() })); +vi.mock("../lib/agent-configuration/load", async importOriginal => ({ + ...await importOriginal(), loadAgentConfiguration: api.load, +})); +vi.mock("../lib/agent-configuration/save", async importOriginal => ({ + ...await importOriginal(), saveAgentConfiguration: api.save, +})); +vi.mock("../context/ToastContext", () => ({ useToastActions: () => ({ pushToast: vi.fn() }) })); + +const agent = { + id: "11111111-1111-4111-8111-111111111111", companyId: "company", urlKey: "codie", name: "Codie", + role: "engineer", title: null, icon: null, appearance: appearanceForPalette("bubblegum-sky"), reportsTo: null, + capabilities: null, adapterType: "process", adapterConfig: { command: "echo" }, runtimeConfig: {}, + defaultEnvironmentId: null, budgetMonthlyCents: 0, metadata: null, configurationRevision: "a".repeat(64), + status: "idle", spentMonthlyCents: 0, pauseReason: null, pausedAt: null, + permissions: { canCreateAgents: false }, lastHeartbeatAt: null, createdAt: new Date(), updatedAt: new Date(), +} satisfies Agent & { configurationRevision: string }; + +function snapshot(value = agent) { + return { agent: value, core: value, document: { apiVersion: "paperclip/v1", kind: "Agent", spec: agentConfigurationSpec(value) } }; +} + +describe("Advanced agent configuration", () => { + let container: HTMLDivElement, root: Root, client: QueryClient; + const dirty = vi.fn(), onSaved = vi.fn(); + beforeEach(() => { + vi.clearAllMocks(); + Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + vi.stubGlobal("ResizeObserver", class { observe() {} disconnect() {} unobserve() {} }); + api.load.mockResolvedValue(snapshot()); + container = document.createElement("div"); document.body.appendChild(container); + root = createRoot(container); + client = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false } } }); + }); + afterEach(async () => { await act(() => root.unmount()); client.clear(); container.remove(); vi.unstubAllGlobals(); }); + async function settle(assertion: () => void) { + for (let i = 0; i < 30; i++) { + await act(async () => { await new Promise(resolve => setTimeout(resolve, 5)); }); + try { assertion(); return; } catch (error) { if (i === 29) throw error; } + } + } + async function render() { + await act(() => root.render( + + )); + } + function editor() { return EditorView.findFromDOM(container.querySelector(".cm-content")!)!; } + async function edit(transform: (source: string) => string) { + await act(() => { const view = editor(); view.dispatch({ changes: { from: 0, to: view.state.doc.length, insert: transform(view.state.doc.toString()) } }); }); + } + function button(label: string) { return [...container.querySelectorAll("button")].find(button => button.textContent === label)!; } + + it("retains an invalid or rejected draft and reloads the saved configuration explicitly", async () => { + api.save.mockRejectedValue(new Error("Agent configuration changed while you were editing.")); + await render(); await settle(() => expect(editor()).toBeTruthy()); + await edit(source => source.replace("budgetMonthlyCents: 0", "budgetMonthlyCents: -1")); + expect(button("Save changes").disabled).toBe(true); + expect(container.querySelector('[role="alert"]')?.textContent).toContain("spec.budgetMonthlyCents"); + await edit(source => source.replace("budgetMonthlyCents: -1", "budgetMonthlyCents: 500")); + await act(() => button("Save changes").click()); + await settle(() => expect(container.textContent).toContain("configuration changed while you were editing")); + expect(editor().state.doc.toString()).toContain("budgetMonthlyCents: 500"); + expect(api.save.mock.calls[0][1].spec.budgetMonthlyCents).toBe(500); + await act(() => button("Reload saved").click()); + await settle(() => expect(editor().state.doc.toString()).toContain("budgetMonthlyCents: 0")); + expect(button("Save changes").disabled).toBe(true); + }); + + it("keeps the saved server result and uses its revision for the next edit", async () => { + api.save.mockResolvedValue(snapshot({ ...agent, name: "Codie Two", urlKey: "codie-two", configurationRevision: "b".repeat(64) })); + await render(); await settle(() => expect(editor()).toBeTruthy()); + await edit(source => source.replace("name: Codie", "name: Codie Two")); + await act(() => button("Save changes").click()); + await settle(() => expect(onSaved).toHaveBeenCalledTimes(1)); + expect(button("Save changes").disabled).toBe(true); + expect(editor().state.doc.toString()).toContain("name: Codie Two"); + await edit(source => source.replace("title: null", "title: Engineer")); + await act(() => button("Save changes").click()); + await settle(() => expect(api.save).toHaveBeenCalledTimes(2)); + expect(api.save.mock.calls[1][0].core.configurationRevision).toBe("b".repeat(64)); + expect(api.save.mock.calls[1][1].spec.title).toBe("Engineer"); + }); + + it("blocks retries after partial saves and keeps one-time credentials outside YAML", async () => { + api.save.mockImplementation(async (_snapshot, _document, onReceipt) => { + onReceipt({ id: "key", label: "Automation", token: "one-time-key" }); + throw new ConfigurationSaveInterrupted("second key", ["first key"], new Error("Network lost")); + }); + await render(); await settle(() => expect(editor()).toBeTruthy()); + await edit(source => source.replace("title: null", "title: Engineer")); + await act(() => button("Save changes").click()); + await settle(() => expect(container.textContent).toContain("Reload saved before saving again")); + expect(button("Save changes").disabled).toBe(true); + expect((container.querySelector('input[aria-label="Automation token"]') as HTMLInputElement).value).toBe("one-time-key"); + expect(editor().state.doc.toString()).not.toContain("one-time-key"); + await edit(source => source.replace("title: Engineer", "title: Lead")); + expect(button("Save changes").disabled).toBe(true); + await act(() => button("Reload saved").click()); + await settle(() => expect(editor().state.doc.toString()).toContain("title: null")); + await edit(source => source.replace("title: null", "title: Lead")); + expect(button("Save changes").disabled).toBe(false); + }); + + it("previews changes and sends only one save for a double click", async () => { + 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")); + expect(container.querySelector("summary")?.textContent).toBe("1 change to apply"); + expect(container.querySelector("details")?.textContent).toContain("Agent configuration"); + const view = editor(); + await act(() => { button("Save changes").click(); button("Save changes").click(); }); + await settle(() => expect(api.save).toHaveBeenCalledTimes(1)); + expect(button("Reload saved").disabled).toBe(true); + await act(async () => complete(snapshot())); + await settle(() => expect(button("Save changes").disabled).toBe(true)); + expect(editor()).toBe(view); + }); + + it("validates read-only sections before enabling Save", async () => { + api.load.mockResolvedValue({ ...snapshot(), document: { ...snapshot().document, observed: { apiKeys: [] } } }); + await render(); await settle(() => expect(editor()).toBeTruthy()); + await edit(source => source.replace("apiKeys: []", "apiKeys: [changed]")); + expect(button("Save changes").disabled).toBe(true); + expect(container.textContent).toContain("observed is read-only"); + expect(api.save).not.toHaveBeenCalled(); + }); + + it("keeps Save disabled for formatting-only changes", async () => { + await render(); await settle(() => expect(editor()).toBeTruthy()); + await edit(source => `${source}\n# A local note\n`); + expect(button("Save changes").disabled).toBe(true); + expect(container.textContent).toContain("No configuration changes"); + expect(api.save).not.toHaveBeenCalled(); + }); + + it("shows a configuration read denial without rendering an empty editable document", async () => { + api.load.mockRejectedValue(new Error("Configuration access denied")); + await render(); await settle(() => expect(container.textContent).toContain("Configuration access denied")); + expect(container.querySelector(".cm-content")).toBeNull(); + expect(api.save).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/src/pages/AgentAdvancedTab.tsx b/ui/src/pages/AgentAdvancedTab.tsx new file mode 100644 index 00000000000..efd6bf629fc --- /dev/null +++ b/ui/src/pages/AgentAdvancedTab.tsx @@ -0,0 +1,184 @@ +import { useEffect, useMemo, useRef, useState } from "react"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { basicSetup } from "codemirror"; +import { Compartment, EditorState } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import { yaml } from "@codemirror/lang-yaml"; +import { HighlightStyle, syntaxHighlighting } from "@codemirror/language"; +import { tags } from "@lezer/highlight"; +import type { Agent } from "@paperclipai/shared"; +import { Button } from "../components/ui/button"; +import { useToastActions } from "../context/ToastContext"; +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"; + +const yamlHighlightStyle = HighlightStyle.define([ + { tag: tags.propertyName, class: "text-primary" }, + { tag: tags.string, class: "text-foreground" }, + { tag: [tags.bool, tags.number, tags.null], class: "text-muted-foreground" }, + { tag: tags.keyword, class: "text-primary" }, + { tag: tags.comment, class: "text-muted-foreground" }, +]); + +export interface AgentAdvancedTabProps { + agent: Agent; + onDirtyChange: (dirty: boolean) => void; + onSaved: (updated: Agent) => void; + onSavingChange?: (saving: boolean) => void; +} + +export function AgentAdvancedTab(props: AgentAdvancedTabProps) { + const [initial, setInitial] = useState(null); + const query = useQuery({ + queryKey: ["agent-configuration-yaml", props.agent.companyId, props.agent.id], + queryFn: () => loadAgentConfiguration(props.agent.id, props.agent.companyId), + refetchOnWindowFocus: false, + }); + useEffect(() => { + if (!initial && query.data && !query.isFetching) setInitial(query.data); + }, [initial, query.data, query.isFetching]); + if (!initial) { + if (query.error) return
+

{query.error.message}

+ +
; + return

Loading configuration…

; + } + return ; +} + +function ConfigurationEditor({ agent, initial, onDirtyChange, onSaved, onSavingChange }: AgentAdvancedTabProps & { initial: AgentConfigurationSnapshot }) { + const queryClient = useQueryClient(); + const { pushToast } = useToastActions(); + const [saved, setSaved] = useState(initial); + const [source, setSource] = useState(() => serializeAgentConfigurationDocument(initial.document)); + const [error, setError] = useState(null); + const [reloadRequired, setReloadRequired] = useState(false); + const [progress, setProgress] = useState(""); + const [receipts, setReceipts] = useState([]); + const operationPending = useRef(false); + const active = useRef(true); + useEffect(() => { active.current = true; return () => { active.current = false; }; }, []); + const original = useMemo(() => serializeAgentConfigurationDocument(saved.document), [saved]); + const dirty = source !== original; + const validation = useMemo(() => { + try { + const document = parseAgentConfigurationYaml(source); + return { document, changes: planAgentConfiguration(saved, document).map(step => step.label), error: null }; + } catch (cause) { return { document: null, changes: [], error: cause instanceof Error ? cause.message : "Invalid YAML" }; } + }, [source, saved]); + useEffect(() => { onDirtyChange(dirty || reloadRequired); }, [dirty, reloadRequired, onDirtyChange]); + useEffect(() => () => onDirtyChange(false), [onDirtyChange]); + + const save = useMutation({ + mutationFn: async () => { + if (!validation.document) throw new Error(validation.error ?? "Invalid YAML"); + if (reloadRequired) throw new Error("Reload saved before saving again."); + return saveAgentConfiguration(saved, validation.document, + receipt => { if (active.current) setReceipts(current => [...current, receipt]); }, + label => { if (active.current) setProgress(label); }); + }, + retry: false, + onSuccess: configuration => { + queryClient.invalidateQueries({ queryKey: queryKeys.agents.detail(agent.id) }); + queryClient.invalidateQueries({ queryKey: queryKeys.agents.detail(agent.urlKey) }); + queryClient.invalidateQueries({ queryKey: queryKeys.agents.list(agent.companyId) }); + queryClient.setQueryData(["agent-configuration-yaml", agent.companyId, agent.id], configuration); + if (!active.current) return; + setSaved(configuration); + setSource(serializeAgentConfigurationDocument(configuration.document)); + setError(null); + onDirtyChange(false); + setReloadRequired(false); + onSaved(configuration.agent); + const warnings = configuration.agent.configurationWarnings; + pushToast({ title: "Agent saved", tone: warnings?.length ? "warn" : "success", body: warnings?.map(warning => warning.message).join("\n") }); + }, + onSettled: () => { + operationPending.current = false; + queryClient.invalidateQueries({ predicate: query => query.queryKey.some(part => part === agent.id || part === agent.companyId), refetchType: "none" }); + }, + onError: cause => { + if (!active.current) return; + setError(cause.message); + if (cause instanceof ConfigurationSaveInterrupted) setReloadRequired(true); + }, + }); + const reload = useMutation({ + mutationFn: () => loadAgentConfiguration(agent.id, agent.companyId), + onSuccess: updated => { + queryClient.setQueryData(["agent-configuration-yaml", agent.companyId, agent.id], updated); + 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); } + }, + onSettled: () => { operationPending.current = false; }, + onError: cause => setError(cause.message), + }); + const busy = save.isPending || reload.isPending; + useEffect(() => { onSavingChange?.(busy); }, [busy, onSavingChange]); + useEffect(() => () => onSavingChange?.(false), [onSavingChange]); + return
+ {receipts.length > 0 &&
+ {receipts.map((receipt, index) =>
+

{receipt.label}

+ {receipt.token ? <> +

Copy this key now. It won’t be shown again after leaving this page.

+ + :

{receipt.id}

} +
)} +
} + { setSource(value); if (!reloadRequired) setError(null); }} readOnly={busy} /> + {dirty && validation.changes.length > 0 &&
+ {validation.changes.length} {validation.changes.length === 1 ? "change" : "changes"} to apply +
    {validation.changes.map((label, index) =>
  • {label}
    • )}
+

Changes save in this order. If one fails, earlier changes remain saved.

+
} + {(validation.error || error) &&

{validation.error || error}

} +
+ +
+ {save.isPending ? `Saving: ${progress || "configuration"}…` : reload.isPending ? "Loading configuration…" : dirty ? validation.document && !validation.changes.length ? "No configuration changes" : "Unsaved changes" : ""} + +
+
+
; +} + +function AgentYamlEditor({ value, onChange, readOnly }: { value: string; onChange: (value: string) => void; readOnly: boolean }) { + const container = useRef(null); + const editor = useRef(null); + const editability = useRef(new Compartment()); + const change = useRef(onChange); + change.current = onChange; + const current = useRef(value); + current.current = value; + useEffect(() => { + if (!container.current) return; + const view = new EditorView({ + parent: container.current, + state: EditorState.create({ doc: current.current, extensions: [ + basicSetup, yaml(), + syntaxHighlighting(yamlHighlightStyle), + EditorView.contentAttributes.of({ "aria-label": "Agent YAML configuration", spellcheck: "false" }), + editability.current.of([EditorState.readOnly.of(readOnly), EditorView.editable.of(!readOnly)]), + EditorView.updateListener.of(update => { if (update.docChanged) change.current(update.state.doc.toString()); }), + ] }), + }); + editor.current = view; + return () => { view.destroy(); editor.current = null; }; + }, []); + useEffect(() => { + editor.current?.dispatch({ effects: editability.current.reconfigure([ + EditorState.readOnly.of(readOnly), EditorView.editable.of(!readOnly), + ]) }); + }, [readOnly]); + useEffect(() => { + const view = editor.current; + if (view && view.state.doc.toString() !== value) view.dispatch({ changes: { from: 0, to: view.state.doc.length, insert: value } }); + }, [value]); + return
; +} diff --git a/ui/src/pages/AgentDetail.tsx b/ui/src/pages/AgentDetail.tsx index 02dbb091e76..49f53a7af89 100644 --- a/ui/src/pages/AgentDetail.tsx +++ b/ui/src/pages/AgentDetail.tsx @@ -14,7 +14,7 @@ import { AgentCharacter } from "../components/AgentCharacter"; import { characterStateForAgent } from "@paperclipai/shared"; import { mergeRunLogChunks, readChunkSeq } from "../lib/run-log-chunks"; import { usePageVisibility } from "../lib/page-visibility"; -import { useCallback, useEffect, useMemo, useState, useRef } from "react"; +import { lazy, Suspense, useCallback, useEffect, useMemo, useState, useRef } from "react"; import { useParams, useNavigate, useLocation, Link, Navigate, useBeforeUnload, type NavigateFunction } from "@/lib/router"; import { useQuery, useMutation, useQueryClient, type QueryClient } from "@tanstack/react-query"; import { @@ -155,6 +155,8 @@ import { type AgentDetailView, } from "./agent-detail-navigation"; +const AgentAdvancedTab = lazy(() => import("./AgentAdvancedTab").then(module => ({ default: module.AgentAdvancedTab }))); + const runStatusIcons: Record = { succeeded: { icon: CheckCircle2, color: "text-green-600 dark:text-green-400" }, failed: { icon: XCircle, color: "text-red-600 dark:text-red-400" }, @@ -821,8 +823,9 @@ export function AgentDetail() { const setSaveConfigAction = useCallback((fn: (() => void) | null) => { saveConfigActionRef.current = fn; }, []); const setCancelConfigAction = useCallback((fn: (() => void) | null) => { cancelConfigActionRef.current = fn; }, []); const prepareAgentNavigation = useCallback(() => { + if (activeView === "advanced" && configSaving) return false; return confirmAgentConfigNavigation(configDirty); - }, [configDirty]); + }, [activeView, configDirty, configSaving]); const agentQuery = useQuery({ queryKey: [...queryKeys.agents.detail(routeAgentRef), lookupCompanyId ?? null], queryFn: () => agentsApi.get(routeAgentRef, lookupCompanyId), @@ -979,6 +982,7 @@ export function AgentDetail() { useEffect(() => { if (!agent) return; + if (activeView === "advanced" && (configSaving || configDirty)) return; if (urlRunId) { if (routeAgentRef !== canonicalAgentRef) { navigate(`/agents/${canonicalAgentRef}/runs/${urlRunId}`, { replace: true }); @@ -991,7 +995,7 @@ export function AgentDetail() { navigate(`${agentDetailHref(canonicalAgentRef, canonicalTab)}${location.search}`, { replace: true }); return; } - }, [agent, routeAgentRef, canonicalAgentRef, urlRunId, urlTab, activeView, legacyAuditSection, navigate, location.search]); + }, [agent, routeAgentRef, canonicalAgentRef, urlRunId, urlTab, activeView, configSaving, configDirty, legacyAuditSection, navigate, location.search]); useEffect(() => { if (!agent?.companyId || agent.companyId === selectedCompanyId) return; @@ -1301,7 +1305,7 @@ export function AgentDetail() { companyId={resolvedCompanyId} assignLabel="Assign Task" showStatus={false} - actionsDisabled={agentAction.isPending} + actionsDisabled={agentAction.isPending || (activeView === "advanced" && configSaving)} workActionsDisabled={hasInvalidOrgChain} workActionsDisabledReason="Repair this agent's reporting chain before assigning tasks or starting runs" hasPendingNavigationChanges={configDirty} @@ -1508,6 +1512,20 @@ export function AgentDetail() { )} + {activeView === "advanced" && ( + Loading configuration…

}> + { + const nextRef = agentRouteRef(updated); + const currentKey = [...queryKeys.agents.detail(routeAgentRef), lookupCompanyId ?? null]; + void queryClient.cancelQueries({ queryKey: currentKey, exact: true }); + queryClient.setQueryData(currentKey, { ...agent, ...updated }); + queryClient.setQueryData([...queryKeys.agents.detail(nextRef), lookupCompanyId ?? null], { ...agent, ...updated }); + if (nextRef !== routeAgentRef) navigate(`/agents/${nextRef}/advanced`, { replace: true }); + }} /> + + )} + {activeView === "run-detail" && ( { "permissions", "api-keys", "revisions", + "advanced", ]); }); it("canonicalizes legacy local paths", () => { + expect(parseAgentDetailView("advanced")).toBe("advanced"); + expect(agentDetailHref("codie", "advanced")).toBe("/agents/codie/advanced"); expect(parseAgentDetailView("dashboard")).toBe("overview"); expect(parseAgentDetailView("configuration")).toBe("runtime"); expect(parseAgentDetailView("prompts")).toBe("instructions"); diff --git a/ui/src/pages/agent-detail-navigation.ts b/ui/src/pages/agent-detail-navigation.ts index 39308751bf3..48d30e8f676 100644 --- a/ui/src/pages/agent-detail-navigation.ts +++ b/ui/src/pages/agent-detail-navigation.ts @@ -11,6 +11,7 @@ export type AgentDetailView = | "permissions" | "api-keys" | "revisions" + | "advanced" | "run-detail"; export type AgentLocalDetailView = Exclude; @@ -42,6 +43,7 @@ export const AGENT_DETAIL_NAVIGATION: ReadonlyArray<{ { value: "permissions", label: "Permissions / Trust" }, { value: "api-keys", label: "API Keys" }, { value: "revisions", label: "Revisions" }, + { value: "advanced", label: "Advanced" }, ], }, ] as const; @@ -56,6 +58,7 @@ export function parseAgentDetailView(value: string | null): AgentLocalDetailView if (value === "permissions" || value === "trust") return "permissions"; if (value === "api-keys" || value === "keys") return "api-keys"; if (value === "revisions" || value === "history") return "revisions"; + if (value === "advanced") return "advanced"; return "overview"; } diff --git a/ui/storybook/prototypes/agent-settings/AgentSettingsPreview.tsx b/ui/storybook/prototypes/agent-settings/AgentSettingsPreview.tsx index b6da8355384..22eca87c84a 100644 --- a/ui/storybook/prototypes/agent-settings/AgentSettingsPreview.tsx +++ b/ui/storybook/prototypes/agent-settings/AgentSettingsPreview.tsx @@ -18,6 +18,7 @@ import { confirmAgentConfigNavigation, } from "@/pages/AgentDetail"; import { AgentSkillsTab } from "@/pages/agent-skills/AgentSkillsTab"; +import { AgentAdvancedTab } from "@/pages/AgentAdvancedTab"; import { AgentToolsTab } from "@/pages/AgentToolsTab"; import { AgentContextualSidebar } from "@/components/AgentContextualSidebar"; import { AgentActionButtons } from "@/components/AgentActionButtons"; @@ -45,6 +46,7 @@ import { storybookIssues } from "../../fixtures/paperclipData"; import "./settings.css"; const descriptions: Record = { + advanced: "Edit the agent’s configuration as YAML.", overview: "A snapshot of Nova’s work, capabilities, and current setup.", instructions: "The files that guide how your agent thinks and works.", skills: "Choose the skills your agent brings to each task.", @@ -275,7 +277,7 @@ function SettingsPage({ runtimeContent }: { runtimeContent?: ReactNode }) { if ( dirty && (event.target as HTMLElement).closest("a") && - !confirmAgentConfigNavigation(true) + (saving || !confirmAgentConfigNavigation(true)) ) { event.preventDefault(); event.stopPropagation(); @@ -471,6 +473,9 @@ function SettingsPage({ runtimeContent }: { runtimeContent?: ReactNode }) { {view === "revisions" && ( )} + {view === "advanced" && ( + {}} /> + )}
{(saveError || permissions.error) && (

diff --git a/ui/storybook/prototypes/agent-settings/fixtures.ts b/ui/storybook/prototypes/agent-settings/fixtures.ts index c5de3920027..e9152f63c78 100644 --- a/ui/storybook/prototypes/agent-settings/fixtures.ts +++ b/ui/storybook/prototypes/agent-settings/fixtures.ts @@ -86,6 +86,8 @@ export function createSettingsFixtures( outcome: TestOutcome = "pass", saveFails = false, ) { + let configurationVersion = 0; + const configurationRevision = () => configurationVersion.toString(16).padStart(64, "0"); let agent: AgentDetail = { ...storybookHiredAgent, id: ID, @@ -117,6 +119,7 @@ export function createSettingsFixtures( }, }; let desiredSkills = ["paperclip", "design-guide"]; + let desiredSkillEntries = desiredSkills.map(key => ({ key, versionId: null as string | null })); const files: Record = { "AGENTS.md": "# Nova\n\nYou are a product engineer. Build interfaces that are clear, useful, and reliable.\n\n## Working style\n\n- Read the task and relevant code before making changes.\n- Keep changes focused and preserve existing behavior.\n- Test the experience in the browser before handing it back.\n\nRead [workflow.md](workflow.md) for your delivery checklist.\n", @@ -155,6 +158,7 @@ export function createSettingsFixtures( supported: true, mode: "ephemeral", desiredSkills, + desiredSkillEntries, warnings: [], entries: desiredSkills.map((key) => ({ key, @@ -331,6 +335,12 @@ export function createSettingsFixtures( } if (url.pathname === `/api/companies/${COMPANY}/tools/connections`) return json({ connections: [connection()] }); + if (url.pathname === `/api/companies/${COMPANY}/tools/profiles`) + return json({ profiles: [] }); + if (url.pathname === `/api/companies/${COMPANY}/chat-endpoints`) + return json([]); + if (url.pathname === `/api/companies/${COMPANY}/secret-proposals`) + return json([]); if (url.pathname === `/api/companies/${COMPANY}/tools/policies`) return json({ policies: [] }); if (url.pathname.includes("/tools/profiles/effective/agents/")) @@ -377,6 +387,11 @@ export function createSettingsFixtures( { status: 503 }, ); const patch = await payload(); + if (patch.expectedConfigurationRevision && patch.expectedConfigurationRevision !== configurationRevision()) { + return Response.json({ error: "Agent configuration changed while you were editing. Reload the saved configuration and apply your changes again." }, { status: 409 }); + } + delete patch.expectedConfigurationRevision; + delete patch.replaceAdapterConfig; const before = agent; agent = path === "/permissions" @@ -391,11 +406,14 @@ export function createSettingsFixtures( } : { ...agent, ...patch }; recordRevision(before, patch); - return json(agent); + configurationVersion++; + return json({ ...agent, configurationRevision: configurationRevision() }); } if (!path) return json(agent); + if (path === "/configuration") return json({ ...agent, configurationRevision: configurationRevision() }); if (path === "/skills/sync") { const data = await payload(); + desiredSkillEntries = data.desiredSkills.map((skill: string | { key: string; versionId?: string }) => typeof skill === "string" ? { key: skill, versionId: null } : { ...skill, versionId: skill.versionId ?? null }); desiredSkills = data.desiredSkills.map( (s: string | { key: string }) => typeof s === "string" ? s : s.key, diff --git a/ui/storybook/stories/agents/agent-settings-redesign.stories.tsx b/ui/storybook/stories/agents/agent-settings-redesign.stories.tsx index 17ff34527b5..9564c92a678 100644 --- a/ui/storybook/stories/agents/agent-settings-redesign.stories.tsx +++ b/ui/storybook/stories/agents/agent-settings-redesign.stories.tsx @@ -37,6 +37,7 @@ const meta = { "permissions", "api-keys", "revisions", + "advanced", ], }, adapterType: { @@ -59,6 +60,13 @@ export default meta; type Story = StoryObj; export const Overview: Story = { args: { initialTab: "overview" } }; export const Instructions: Story = { args: { initialTab: "instructions" } }; +export const Advanced: Story = { + args: { initialTab: "advanced" }, + parameters: { paperclip: { + capabilities: ["agents/instructions"], status: "current", implementation: "mixed", boundary: "fixtures", + notes: "Production YAML editor inside the settings preview shell. Mutable fixtures simulate saves and revision conflicts; server authorization and secret restoration require integration verification.", + } }, +}; export const Skills: Story = { args: { initialTab: "skills" } }; export const Runtime: Story = { name: "Harness / Runtime",