From 7dfb327bb786a6f0a09595770cb821097539a330 Mon Sep 17 00:00:00 2001 From: "Fry (Paperclip)" Date: Sun, 27 Sep 2026 02:27:24 +0000 Subject: [PATCH] feat(instructions): persist canonical revisions with authorized history and restore Implement PAP-66 foundation: exact content history, transactional compare-and-swap saves, current responsible-user authorization, recoverable materialization, and editor history. Keep run staging and cleanup integration with PAP-63. Co-Authored-By: Paperclip --- doc/DATABASE.md | 10 + doc/agent-instruction-revisions.md | 164 ++++ .../migrations/0285_legal_thunderbolts.sql | 32 + ...{0280_snapshot.json => 0285_snapshot.json} | 903 +++++++++++++++++- packages/db/src/migrations/meta/_journal.json | 7 + .../src/schema/agent_instruction_revisions.ts | 84 ++ packages/db/src/schema/index.ts | 2 + packages/shared/src/index.ts | 3 + packages/shared/src/types/agent.ts | 60 ++ packages/shared/src/types/index.ts | 2 + packages/shared/src/validators/agent.ts | 13 +- packages/shared/src/validators/index.ts | 2 + .../agent-instruction-revisions.test.ts | 240 +++++ .../agent-instructions-routes.test.ts | 79 +- .../agent-instructions-service.test.ts | 16 +- .../__tests__/built-in-agent-routes.test.ts | 2 +- server/src/__tests__/built-in-agents.test.ts | 36 +- server/src/routes/agents.ts | 95 +- server/src/routes/built-in-agents.ts | 2 +- server/src/routes/openapi.ts | 14 + .../agent-instruction-authorization.ts | 175 ++++ .../src/services/agent-instruction-files.ts | 149 +++ .../services/agent-instruction-revisions.ts | 469 +++++++++ server/src/services/agent-instructions.ts | 129 ++- server/src/services/authorization.ts | 21 +- server/src/services/built-in-agents.ts | 59 +- server/src/services/change-consent-gate.ts | 2 +- server/src/services/company-portability.ts | 2 +- server/src/services/plugin-managed-agents.ts | 2 +- ui/src/api/agents.ts | 13 +- ui/src/components/InstructionHistory.tsx | 145 +++ .../pages/AgentDetail.instructions.test.tsx | 35 + ui/src/pages/AgentDetail.tsx | 40 +- 33 files changed, 2892 insertions(+), 115 deletions(-) create mode 100644 doc/agent-instruction-revisions.md create mode 100644 packages/db/src/migrations/0285_legal_thunderbolts.sql rename packages/db/src/migrations/meta/{0280_snapshot.json => 0285_snapshot.json} (98%) create mode 100644 packages/db/src/schema/agent_instruction_revisions.ts create mode 100644 server/src/__tests__/agent-instruction-revisions.test.ts create mode 100644 server/src/services/agent-instruction-authorization.ts create mode 100644 server/src/services/agent-instruction-files.ts create mode 100644 server/src/services/agent-instruction-revisions.ts create mode 100644 ui/src/components/InstructionHistory.tsx diff --git a/doc/DATABASE.md b/doc/DATABASE.md index 1d21aa87f81..48321923a79 100644 --- a/doc/DATABASE.md +++ b/doc/DATABASE.md @@ -432,3 +432,13 @@ cleanup authority; it does not prove that remote inference has stopped. Recovery revokes the previous boot identity with a conditional update. Its own claim also expires so another sweep can finish cleanup after a restart. Historical rows keep null ownership fields and follow the previous recovery path. + +## Agent instruction content revisions + +`agent_instruction_revisions` stores immutable exact UTF-8 content and server-bound +attribution. `agent_instruction_heads` selects the current revision by company, +agent, and configured entry filename. Revision/head/activity changes share one +transaction; disk content is a recoverable projection. Existing managed files +are seeded lazily without rewriting their content. See +[Canonical agent instruction revisions](agent-instruction-revisions.md) for the +commit, conflict, authorization, and runtime integration contract. diff --git a/doc/agent-instruction-revisions.md b/doc/agent-instruction-revisions.md new file mode 100644 index 00000000000..126ce8727cf --- /dev/null +++ b/doc/agent-instruction-revisions.md @@ -0,0 +1,164 @@ +# Canonical agent instruction revisions + +Managed entry content has one database history. `agent_instruction_revisions` +stores exact UTF-8 bytes as base64 text (including BOM, CRLF, trailing whitespace, +and NUL), SHA-256, byte length, parent/base revision, restore origin, actor, +responsible user, source run, source, and creation time. `agent_instruction_heads` +selects one revision per company, agent, and relative entry filename. Composite +foreign keys bind heads to their owner's content and agents to their company. +Configuration snapshots in `agent_config_revisions` are not content history. +Migration 0285 adds the tables; existing managed files are seeded lazily on the +first authorized canonical read or commit, without changing their bytes. + +The configured `instructionsEntryFile` is authoritative. Legacy managed +`instructionsFilePath` can select a nested filename. A missing configured entry +never causes a scan to select some other AGENTS.md. Supporting bundle files keep +the existing file API; only the configured entry participates in this history. + +## Service integration + +Import `agentInstructionRevisionService` from +`server/src/services/agent-instruction-revisions.ts` and instantiate it with `db`. +Use the server's authenticated actor, never an actor or responsible-user ID from +request/tool arguments. `AuthorizationActor` is an internal server type. + +```ts +const revisions = agentInstructionRevisionService(db); +const target = { companyId, agentId: targetAgentId }; +const baseline = await revisions.readCurrent(target, serverActor); +// null means no committed head and no existing managed entry. +// For an existing head, retain its entryFile/id/hash in the server-owned mapping. +const receipt = await revisions.commit({ + ...target, + entryFile: baseline!.revision.entryFile, + baseRevisionId: baseline!.revision.id, + content: collectedBytes, // string or Uint8Array, at most 1 MiB of valid UTF-8 + source: "cleanup", // board | api | tool | cleanup +}, serverActor); +``` + +For new entries, obtain the configured entry from `deriveBundleState(agent)` and +pass `baseRevisionId: null`. Do not turn a conflict into a null base or silently +retry with the latest head. `readInstructionBytes(root, entryFile)` from +`agent-instruction-files.ts` validates a specifically registered file; it returns +null for a missing file, rejects symlinks in the path, rejects special files, and +bounds the read itself. The collector must distinguish a missing working file +from a valid empty file and must stop the process before collecting it. + +Public operations: + +- `readCurrent(target, actor)` returns `AgentInstructionSnapshot | null` and seeds + existing managed bytes. Reads do not require content-edit permission. +- `commit(input, actor)` returns `AgentInstructionCommitReceipt`. +- `history({ ...target, entryFile, cursor?, limit? }, actor)` returns metadata and + `nextCursor`; pages are capped at 100 revisions (default 50). +- `readRevision({ ...target, entryFile, revisionId }, actor)` returns exact content. +- `diff({ ...target, entryFile, fromRevisionId, toRevisionId }, actor)` returns both + snapshots and an exact common-prefix/replacement/common-suffix diff in linear + time. It does not attempt an automatic merge. +- `restore({ ...target, entryFile, revisionId, baseRevisionId }, actor)` uses the + same authorization and CAS path. It appends a revision with source `restore` + and `restoredFromRevisionId`. Identical content is a no-op. +- `materializeCurrent(target)` is an **internal recovery operation**, not an + authorization API. It copies only the current committed head to the managed + path while holding the agent lock. Call before launching a run that reads disk + and after restarting following an interrupted materialization. + +A transaction locks the target agent, rechecks current authority, inserts a +revision, moves the head, and inserts its activity record. A different current +head yields 409. Identical content returns the existing receipt without another +revision, including retries whose base has advanced to that same content. +Materialization happens after commit under the same serialization lock, always +from the latest head. It uses a temporary file and atomic rename. A later save +cannot be overwritten by an older materializer. A crash or disk error leaves the +database head recoverable. A receipt with `materialization: "pending"` means the +revision is durable and the disk copy still needs repair; it does not mean the +commit failed. GET bundle also attempts that repair and displays a warning if +repair fails. A successful materializer may have copied a newer head than the +receipt if another commit already completed. + +`authorizeInstructionCommit(connection, actor, target)` and +`resolveInstructionActor(connection, actor)` are exported from +`agent-instruction-authorization.ts`. The commit service calls them itself; +callers must not treat an earlier authorization result as a reusable grant. +Agent callers must supply the server-bound agent/company plus registered run or +active API key. The service reloads responsible identity, accepted active run +identity context, key scope/revocation, user existence, active company membership, +current target edit permission, and agent restrictions. When capturing a baseline, +retain the server-resolved `onBehalfOfUserId` so identity changes before cleanup +fail closed instead of attributing the candidate to a different user. + +Ordinary standard agents inherit their responsible user's existing target +`agents:configure` access for content only. They do not need their own blanket +agent-admin grant. An explicit scoped configure grant or suggest-only grant still +constrains them; suggest-only mutation consumes the existing accepted-change +confirmation inside the commit transaction. Low-trust, task-bridge, and skill-test +containment remains enforced. This path enforces the responsible-user ceiling even +if the general responsible-user policy is configured in shadow mode. + +## HTTP and editor + +Existing company-scoped agent URLs now support: + +| Operation | Route | +| --- | --- | +| Current entry and revision metadata | `GET /api/agents/:id/instructions-bundle/file?path=...` | +| Content save | `PUT /api/agents/:id/instructions-bundle/file` | +| History | `GET /api/agents/:id/instructions-bundle/history?path=...&cursor=...` | +| Revision content | `GET /api/agents/:id/instructions-bundle/revision/:revisionId?path=...` | +| Diff | `GET /api/agents/:id/instructions-bundle/diff?path=...&from=...&to=...` | +| Restore | `POST /api/agents/:id/instructions-bundle/restore` | + +Entry PUT requires `{ path, content, baseRevisionId }`. Restore requires +`{ path, revisionId, baseRevisionId }`. Content requests reject unknown fields, +including supplied responsible identity. Save/restore return the normal file +detail plus `revision` and `receipt`. The existing editor pins a draft's base, +retains edits on conflicts, shows history and exact revision content/differences, +and restores against the currently displayed head. Save or discard local edits +before restoring. Configuration changes (root, entry selection, legacy prompt +removal) retain the configuration permission gate. + +`AgentInstructionSnapshot`, `AgentInstructionRevision`, +`AgentInstructionCommitReceipt`, `AgentInstructionHistory`, +`AgentInstructionDiff`, `AgentInstructionErrorCode`, and +`AgentInstructionErrorDetails` are exported from `@paperclipai/shared`. +Errors use `HttpError` and the existing HTTP error envelope. A conflict may +return a null `currentRevisionId` when the existing disk file has not yet been +seeded; read the current entry to obtain its durable initial revision: + +| Status | Detail code / action | +| --- | --- | +| 409 | `INSTRUCTION_REVISION_CONFLICT`: preserve candidate, read current head, explicitly resolve | +| 409 | `INSTRUCTION_ENTRY_CHANGED`: configured entry changed; do not collect into the replacement entry | +| 403 | `INSTRUCTION_IDENTITY_INVALID`, `RESPONSIBLE_USER_UNAVAILABLE`, `RESPONSIBLE_USER_UNAUTHORIZED`, or existing authorization/consent denial | +| 422 | `INSTRUCTION_BASE_REQUIRED`: read first and provide the base | +| 422 | `INSTRUCTION_CONTENT_INVALID`, `INSTRUCTION_PATH_INVALID`: reject candidate; preserve canonical bytes | +| 422 | `INSTRUCTION_MANAGED_BUNDLE_REQUIRED`: instance administrator must explicitly migrate the external bundle | +| 422 | `INSTRUCTION_REVISION_REQUIRED`: use canonical content commit instead of filesystem overwrite | +| 404 | Target or revision is absent from the requested owner scope | + +External host bundles are never automatically seeded or written by this service. +Their existing instance-admin read/configuration restrictions remain. Managed +bootstrap/import/reset helpers now refuse to overwrite an existing entry with +different content or any entry with revision history. Apply content changes through +the canonical commit path; provisioning a new entry remains supported. These +helpers do not provide an alternate content-history bypass. Supporting-file writes +reload configuration under the same agent lock and refuse current or historical +versioned entries, including when a route has a stale configuration snapshot. +Board stock resets +for built-in agents pass the authenticated actor through the same commit service. +Automatic reconciliation preserves existing stock content when no responsible +operator is present; the available update remains visible for an authorized reset. +Generic import and plugin reset callers must use the canonical writer to replace +existing versioned content; their filesystem initialization helper fails closed. + +## Runtime handoff boundary + +This foundation does not register run-local copies, change native or legacy +staging, dispatch runner tools, collect stopped runs, or persist pending cleanup +candidates. Those integrations must retain the baseline and server identity, +collect the registered private file after stop, use this commit path, preserve +conflicts/loss evidence, and arrange bounded recovery. They must not write shared +instruction caches or call the old filesystem `writeFile` for an entry. +The next run should read the canonical snapshot and materialize it when disk is +needed. Runtime cleanup parity is not established by these service/editor tests. diff --git a/packages/db/src/migrations/0285_legal_thunderbolts.sql b/packages/db/src/migrations/0285_legal_thunderbolts.sql new file mode 100644 index 00000000000..27522ed65a9 --- /dev/null +++ b/packages/db/src/migrations/0285_legal_thunderbolts.sql @@ -0,0 +1,32 @@ +CREATE TABLE "agent_instruction_heads" ( + "company_id" uuid NOT NULL, + "agent_id" uuid NOT NULL, + "entry_file" text NOT NULL, + "revision_id" uuid NOT NULL, + "updated_at" timestamp with time zone DEFAULT now() NOT NULL, + CONSTRAINT "agent_instruction_heads_identity_uq" UNIQUE("company_id","agent_id","entry_file") +); +--> statement-breakpoint +CREATE TABLE "agent_instruction_revisions" ( + "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, + "company_id" uuid NOT NULL, + "agent_id" uuid NOT NULL, + "entry_file" text NOT NULL, + "content_base64" text NOT NULL, + "content_hash" text NOT NULL, + "byte_length" integer NOT NULL, + "parent_revision_id" uuid, + "base_revision_id" uuid, + "restored_from_revision_id" uuid, + "actor_agent_id" uuid, + "actor_user_id" text, + "responsible_user_id" text, + "source_run_id" uuid, + "source" text NOT NULL, + "created_at" timestamp with time zone DEFAULT now() NOT NULL, + CONSTRAINT "agent_instruction_revisions_identity_uq" UNIQUE("company_id","agent_id","entry_file","id") +); +--> statement-breakpoint +ALTER TABLE "agent_instruction_heads" ADD CONSTRAINT "agent_instruction_heads_company_id_agent_id_entry_file_revision_id_agent_instruction_revisions_company_id_agent_id_entry_file_id_fk" FOREIGN KEY ("company_id","agent_id","entry_file","revision_id") REFERENCES "public"."agent_instruction_revisions"("company_id","agent_id","entry_file","id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint +ALTER TABLE "agent_instruction_revisions" ADD CONSTRAINT "agent_instruction_revisions_company_id_agent_id_agents_company_id_id_fk" FOREIGN KEY ("company_id","agent_id") REFERENCES "public"."agents"("company_id","id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint +CREATE INDEX "agent_instruction_revisions_history_idx" ON "agent_instruction_revisions" USING btree ("company_id","agent_id","entry_file","created_at","id"); \ No newline at end of file diff --git a/packages/db/src/migrations/meta/0280_snapshot.json b/packages/db/src/migrations/meta/0285_snapshot.json similarity index 98% rename from packages/db/src/migrations/meta/0280_snapshot.json rename to packages/db/src/migrations/meta/0285_snapshot.json index ea3ae5e50dd..1ebca7b5290 100644 --- a/packages/db/src/migrations/meta/0280_snapshot.json +++ b/packages/db/src/migrations/meta/0285_snapshot.json @@ -1,6 +1,6 @@ { - "id": "861a2815-1418-4c71-a77b-d0e2e9566413", - "prevId": "4770ad6d-2cdc-45c4-af89-c300fd134d91", + "id": "242cfd69-e9ef-4220-8bab-9d98d199acbe", + "prevId": "7db2ceec-ed62-4968-90c9-4c79206cfe34", "version": "7", "dialect": "postgresql", "tables": { @@ -829,6 +829,258 @@ "checkConstraints": {}, "isRLSEnabled": false }, + "public.agent_instruction_heads": { + "name": "agent_instruction_heads", + "schema": "", + "columns": { + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "agent_id": { + "name": "agent_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "entry_file": { + "name": "entry_file", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "revision_id": { + "name": "revision_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "agent_instruction_heads_company_id_agent_id_entry_file_revision_id_agent_instruction_revisions_company_id_agent_id_entry_file_id_fk": { + "name": "agent_instruction_heads_company_id_agent_id_entry_file_revision_id_agent_instruction_revisions_company_id_agent_id_entry_file_id_fk", + "tableFrom": "agent_instruction_heads", + "tableTo": "agent_instruction_revisions", + "columnsFrom": [ + "company_id", + "agent_id", + "entry_file", + "revision_id" + ], + "columnsTo": [ + "company_id", + "agent_id", + "entry_file", + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "agent_instruction_heads_identity_uq": { + "name": "agent_instruction_heads_identity_uq", + "nullsNotDistinct": false, + "columns": [ + "company_id", + "agent_id", + "entry_file" + ] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.agent_instruction_revisions": { + "name": "agent_instruction_revisions", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "agent_id": { + "name": "agent_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "entry_file": { + "name": "entry_file", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "content_base64": { + "name": "content_base64", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "content_hash": { + "name": "content_hash", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "byte_length": { + "name": "byte_length", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "parent_revision_id": { + "name": "parent_revision_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "base_revision_id": { + "name": "base_revision_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "restored_from_revision_id": { + "name": "restored_from_revision_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "actor_agent_id": { + "name": "actor_agent_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "actor_user_id": { + "name": "actor_user_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "responsible_user_id": { + "name": "responsible_user_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "source_run_id": { + "name": "source_run_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "source": { + "name": "source", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "agent_instruction_revisions_history_idx": { + "name": "agent_instruction_revisions_history_idx", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "agent_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "entry_file", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "created_at", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "agent_instruction_revisions_company_id_agent_id_agents_company_id_id_fk": { + "name": "agent_instruction_revisions_company_id_agent_id_agents_company_id_id_fk", + "tableFrom": "agent_instruction_revisions", + "tableTo": "agents", + "columnsFrom": [ + "company_id", + "agent_id" + ], + "columnsTo": [ + "company_id", + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "agent_instruction_revisions_identity_uq": { + "name": "agent_instruction_revisions_identity_uq", + "nullsNotDistinct": false, + "columns": [ + "company_id", + "agent_id", + "entry_file", + "id" + ] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, "public.agent_memberships": { "name": "agent_memberships", "schema": "", @@ -3302,6 +3554,13 @@ "primaryKey": false, "notNull": false }, + "keyboard_shortcuts": { + "name": "keyboard_shortcuts", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": false + }, "created_at": { "name": "created_at", "type": "timestamp with time zone", @@ -5701,6 +5960,12 @@ "notNull": true, "default": false }, + "communication_guidance": { + "name": "communication_guidance", + "type": "text", + "primaryKey": false, + "notNull": false + }, "state": { "name": "state", "type": "text", @@ -6611,6 +6876,13 @@ "primaryKey": false, "notNull": false }, + "communication_instructions": { + "name": "communication_instructions", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "''" + }, "allow_direct_messages": { "name": "allow_direct_messages", "type": "boolean", @@ -8096,6 +8368,519 @@ }, "isRLSEnabled": false }, + "public.chat_github_configurations": { + "name": "chat_github_configurations", + "schema": "", + "columns": { + "endpoint_id": { + "name": "endpoint_id", + "type": "uuid", + "primaryKey": true, + "notNull": true + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "revision": { + "name": "revision", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 1 + }, + "configuration": { + "name": "configuration", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "updated_by_user_id": { + "name": "updated_by_user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "chat_github_configurations_company_id_endpoint_id_chat_endpoints_company_id_id_fk": { + "name": "chat_github_configurations_company_id_endpoint_id_chat_endpoints_company_id_id_fk", + "tableFrom": "chat_github_configurations", + "tableTo": "chat_endpoints", + "columnsFrom": [ + "company_id", + "endpoint_id" + ], + "columnsTo": [ + "company_id", + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": { + "chat_github_config_revision_check": { + "name": "chat_github_config_revision_check", + "value": "\"chat_github_configurations\".\"revision\" > 0" + } + }, + "isRLSEnabled": false + }, + "public.chat_github_registrations": { + "name": "chat_github_registrations", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "endpoint_id": { + "name": "endpoint_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "state_hash": { + "name": "state_hash", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "trusted_origin": { + "name": "trusted_origin", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "'pending'" + }, + "expires_at": { + "name": "expires_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true + }, + "consumed_at": { + "name": "consumed_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "chat_github_registration_state_uq": { + "name": "chat_github_registration_state_uq", + "columns": [ + { + "expression": "state_hash", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + }, + "chat_github_registration_endpoint_idx": { + "name": "chat_github_registration_endpoint_idx", + "columns": [ + { + "expression": "endpoint_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "expires_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "chat_github_registrations_company_id_endpoint_id_chat_endpoints_company_id_id_fk": { + "name": "chat_github_registrations_company_id_endpoint_id_chat_endpoints_company_id_id_fk", + "tableFrom": "chat_github_registrations", + "tableTo": "chat_endpoints", + "columnsFrom": [ + "company_id", + "endpoint_id" + ], + "columnsTo": [ + "company_id", + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": { + "chat_github_registration_status_check": { + "name": "chat_github_registration_status_check", + "value": "\"chat_github_registrations\".\"status\" in ('pending', 'exchanging', 'completed', 'failed')" + } + }, + "isRLSEnabled": false + }, + "public.chat_github_reviews": { + "name": "chat_github_reviews", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "endpoint_id": { + "name": "endpoint_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "issue_id": { + "name": "issue_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "run_id": { + "name": "run_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "repository_id": { + "name": "repository_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "repository": { + "name": "repository", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "pull_number": { + "name": "pull_number", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "head_sha": { + "name": "head_sha", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "delivery_id": { + "name": "delivery_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "configuration_revision": { + "name": "configuration_revision", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "policy_snapshot": { + "name": "policy_snapshot", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "event": { + "name": "event", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "state": { + "name": "state", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "'queued'" + }, + "assessment": { + "name": "assessment", + "type": "jsonb", + "primaryKey": false, + "notNull": false + }, + "conclusion": { + "name": "conclusion", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "check_id": { + "name": "check_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "check_url": { + "name": "check_url", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "summary_id": { + "name": "summary_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "summary_url": { + "name": "summary_url", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "publication_receipts": { + "name": "publication_receipts", + "type": "jsonb", + "primaryKey": false, + "notNull": true, + "default": "'{}'::jsonb" + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "chat_github_reviews_delivery_uq": { + "name": "chat_github_reviews_delivery_uq", + "columns": [ + { + "expression": "endpoint_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "delivery_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + }, + "chat_github_reviews_pull_idx": { + "name": "chat_github_reviews_pull_idx", + "columns": [ + { + "expression": "endpoint_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "repository_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "pull_number", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "created_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, + "chat_github_reviews_task_idx": { + "name": "chat_github_reviews_task_idx", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "issue_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "run_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "chat_github_reviews_run_id_heartbeat_runs_id_fk": { + "name": "chat_github_reviews_run_id_heartbeat_runs_id_fk", + "tableFrom": "chat_github_reviews", + "tableTo": "heartbeat_runs", + "columnsFrom": [ + "run_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "set null", + "onUpdate": "no action" + }, + "chat_github_reviews_company_id_endpoint_id_chat_endpoints_company_id_id_fk": { + "name": "chat_github_reviews_company_id_endpoint_id_chat_endpoints_company_id_id_fk", + "tableFrom": "chat_github_reviews", + "tableTo": "chat_endpoints", + "columnsFrom": [ + "company_id", + "endpoint_id" + ], + "columnsTo": [ + "company_id", + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "chat_github_reviews_company_id_issue_id_issues_company_id_id_fk": { + "name": "chat_github_reviews_company_id_issue_id_issues_company_id_id_fk", + "tableFrom": "chat_github_reviews", + "tableTo": "issues", + "columnsFrom": [ + "company_id", + "issue_id" + ], + "columnsTo": [ + "company_id", + "id" + ], + "onDelete": "restrict", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "chat_github_reviews_company_id_uq": { + "name": "chat_github_reviews_company_id_uq", + "nullsNotDistinct": false, + "columns": [ + "company_id", + "id" + ] + } + }, + "policies": {}, + "checkConstraints": { + "chat_github_review_pull_number_check": { + "name": "chat_github_review_pull_number_check", + "value": "\"chat_github_reviews\".\"pull_number\" > 0" + }, + "chat_github_review_state_check": { + "name": "chat_github_review_state_check", + "value": "\"chat_github_reviews\".\"state\" in ('queued', 'running', 'completed', 'incomplete', 'error', 'superseded', 'manual_required')" + } + }, + "isRLSEnabled": false + }, "public.chat_teams_file_transfers": { "name": "chat_teams_file_transfers", "schema": "", @@ -37416,6 +38201,26 @@ "notNull": true, "default": true }, + "setup_pending": { + "name": "setup_pending", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": false + }, + "archived": { + "name": "archived", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": false + }, + "last_webhook_delivery": { + "name": "last_webhook_delivery", + "type": "jsonb", + "primaryKey": false, + "notNull": false + }, "cron_expression": { "name": "cron_expression", "type": "text", @@ -37677,6 +38482,100 @@ "checkConstraints": {}, "isRLSEnabled": false }, + "public.routine_webhook_test_receipts": { + "name": "routine_webhook_test_receipts", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "trigger_id": { + "name": "trigger_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "delivery_key_hash": { + "name": "delivery_key_hash", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "received_at": { + "name": "received_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "routine_webhook_test_receipts_delivery_uq": { + "name": "routine_webhook_test_receipts_delivery_uq", + "columns": [ + { + "expression": "trigger_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "delivery_key_hash", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "routine_webhook_test_receipts_company_id_companies_id_fk": { + "name": "routine_webhook_test_receipts_company_id_companies_id_fk", + "tableFrom": "routine_webhook_test_receipts", + "tableTo": "companies", + "columnsFrom": [ + "company_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "routine_webhook_test_receipts_trigger_id_routine_triggers_id_fk": { + "name": "routine_webhook_test_receipts_trigger_id_routine_triggers_id_fk", + "tableFrom": "routine_webhook_test_receipts", + "tableTo": "routine_triggers", + "columnsFrom": [ + "trigger_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, "public.routines": { "name": "routines", "schema": "", diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index a46ca4cfb44..b5de2bf5add 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -1982,6 +1982,13 @@ "when": 1790439010225, "tag": "0284_petite_genesis", "breakpoints": true + }, + { + "idx": 285, + "version": "7", + "when": 1790474441205, + "tag": "0285_legal_thunderbolts", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/db/src/schema/agent_instruction_revisions.ts b/packages/db/src/schema/agent_instruction_revisions.ts new file mode 100644 index 00000000000..f550c74c4db --- /dev/null +++ b/packages/db/src/schema/agent_instruction_revisions.ts @@ -0,0 +1,84 @@ +import { + pgTable, + uuid, + text, + integer, + timestamp, + index, + unique, + foreignKey, +} from "drizzle-orm/pg-core"; +import { agents } from "./agents.js"; + +/** Append-only content. Base64 preserves every UTF-8 byte, including NUL and BOM. */ +export const agentInstructionRevisions = pgTable( + "agent_instruction_revisions", + { + id: uuid("id").primaryKey().defaultRandom(), + companyId: uuid("company_id").notNull(), + agentId: uuid("agent_id").notNull(), + entryFile: text("entry_file").notNull(), + contentBase64: text("content_base64").notNull(), + contentHash: text("content_hash").notNull(), + byteLength: integer("byte_length").notNull(), + parentRevisionId: uuid("parent_revision_id"), + baseRevisionId: uuid("base_revision_id"), + restoredFromRevisionId: uuid("restored_from_revision_id"), + actorAgentId: uuid("actor_agent_id"), + actorUserId: text("actor_user_id"), + responsibleUserId: text("responsible_user_id"), + sourceRunId: uuid("source_run_id"), + source: text("source").notNull(), + createdAt: timestamp("created_at", { withTimezone: true }) + .notNull() + .defaultNow(), + }, + (t) => ({ + owner: foreignKey({ + columns: [t.companyId, t.agentId], + foreignColumns: [agents.companyId, agents.id], + }).onDelete("cascade"), + identity: unique("agent_instruction_revisions_identity_uq").on( + t.companyId, + t.agentId, + t.entryFile, + t.id, + ), + history: index("agent_instruction_revisions_history_idx").on( + t.companyId, + t.agentId, + t.entryFile, + t.createdAt, + t.id, + ), + }), +); + +export const agentInstructionHeads = pgTable( + "agent_instruction_heads", + { + companyId: uuid("company_id").notNull(), + agentId: uuid("agent_id").notNull(), + entryFile: text("entry_file").notNull(), + revisionId: uuid("revision_id").notNull(), + updatedAt: timestamp("updated_at", { withTimezone: true }) + .notNull() + .defaultNow(), + }, + (t) => ({ + identity: unique("agent_instruction_heads_identity_uq").on( + t.companyId, + t.agentId, + t.entryFile, + ), + revision: foreignKey({ + columns: [t.companyId, t.agentId, t.entryFile, t.revisionId], + foreignColumns: [ + agentInstructionRevisions.companyId, + agentInstructionRevisions.agentId, + agentInstructionRevisions.entryFile, + agentInstructionRevisions.id, + ], + }).onDelete("cascade"), + }), +); diff --git a/packages/db/src/schema/index.ts b/packages/db/src/schema/index.ts index 81ea3bc33bb..305033b6366 100644 --- a/packages/db/src/schema/index.ts +++ b/packages/db/src/schema/index.ts @@ -210,3 +210,5 @@ export { aiConnectionDefaults } from "./ai_connection_defaults.js"; export { aiProviderDefaults } from "./ai_provider_defaults.js"; export * from "./email.js"; export { announcementDismissals, announcementPublications } from "./announcement_dismissals.js"; + +export { agentInstructionRevisions, agentInstructionHeads } from "./agent_instruction_revisions.js"; diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 9ae73b45045..15f458a5e4c 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -2788,3 +2788,6 @@ export { MEMORY_CONNECTOR_IDS, isMemoryConnectorId, type MemoryConnectorId } fro export * from "./connection-routing.js"; export { WORKSPACE_RESTORE_FAILURE_CODES, hasWorkspaceRestoreFailure, safeWorkspaceRestorePath } from "./workspace-restore.js"; + +export type { AgentInstructionErrorCode, AgentInstructionErrorDetails, AgentInstructionSource, AgentInstructionRevision, AgentInstructionSnapshot, AgentInstructionCommitReceipt, AgentInstructionHistory, AgentInstructionDiff } from "./types/agent.js"; +export { restoreAgentInstructionSchema } from "./validators/agent.js"; diff --git a/packages/shared/src/types/agent.ts b/packages/shared/src/types/agent.ts index e775c5b9941..cda07d1c7b2 100644 --- a/packages/shared/src/types/agent.ts +++ b/packages/shared/src/types/agent.ts @@ -43,6 +43,8 @@ export interface AgentInstructionsFileSummary { export interface AgentInstructionsFileDetail extends AgentInstructionsFileSummary { content: string; + revision?: AgentInstructionRevision; + receipt?: AgentInstructionCommitReceipt; } export interface AgentInstructionsBundle { @@ -336,3 +338,61 @@ export type AdapterAuthSignal = "present" | "absent" | "unknown"; export interface AdapterAuthSignalResponse { status: AdapterAuthSignal; } + +export type AgentInstructionSource = "seed" | "board" | "api" | "tool" | "cleanup" | "restore"; +export interface AgentInstructionRevision { + id: string; + companyId: string; + agentId: string; + entryFile: string; + contentHash: string; + byteLength: number; + parentRevisionId: string | null; + baseRevisionId: string | null; + restoredFromRevisionId: string | null; + actorAgentId: string | null; + actorUserId: string | null; + responsibleUserId: string | null; + sourceRunId: string | null; + source: AgentInstructionSource; + createdAt: string; +} +export interface AgentInstructionSnapshot { + revision: AgentInstructionRevision; + content: string; +} +export interface AgentInstructionCommitReceipt extends AgentInstructionSnapshot { + changed: boolean; + materialization: "current" | "pending"; +} +export interface AgentInstructionHistory { + revisions: AgentInstructionRevision[]; + nextCursor: string | null; +} +export interface AgentInstructionDiff { + from: AgentInstructionSnapshot; + to: AgentInstructionSnapshot; + prefix: string; + removed: string; + added: string; + suffix: string; +} + +export type AgentInstructionErrorCode = + | "INSTRUCTION_BASE_REQUIRED" + | "INSTRUCTION_REVISION_REQUIRED" + | "INSTRUCTION_REVISION_CONFLICT" + | "INSTRUCTION_ENTRY_CHANGED" + | "INSTRUCTION_MANAGED_BUNDLE_REQUIRED" + | "INSTRUCTION_IDENTITY_INVALID" + | "INSTRUCTION_CONTENT_INVALID" + | "INSTRUCTION_PATH_INVALID" + | "RESPONSIBLE_USER_UNAVAILABLE" + | "RESPONSIBLE_USER_UNAUTHORIZED"; +export interface AgentInstructionErrorDetails { + code: AgentInstructionErrorCode; + baseRevisionId?: string | null; + currentRevisionId?: string | null; + entryFile?: string; + reason?: string; +} diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index 179832a3de4..b9dd9c280c0 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -1069,3 +1069,5 @@ export * from "./chat-channels.js"; export * from "./chat-github.js"; export * from "./email.js"; + +export type { AgentInstructionErrorCode, AgentInstructionErrorDetails, AgentInstructionSource, AgentInstructionRevision, AgentInstructionSnapshot, AgentInstructionCommitReceipt, AgentInstructionHistory, AgentInstructionDiff } from "./agent.js"; diff --git a/packages/shared/src/validators/agent.ts b/packages/shared/src/validators/agent.ts index e40a7cf6ce7..3086c4f30ae 100644 --- a/packages/shared/src/validators/agent.ts +++ b/packages/shared/src/validators/agent.ts @@ -34,10 +34,17 @@ export const updateAgentInstructionsBundleSchema = z.object({ export type UpdateAgentInstructionsBundle = z.infer; export const upsertAgentInstructionsFileSchema = z.object({ - path: z.string().trim().min(1), - content: z.string(), + path: z.string().min(1).max(512), + content: z.string().max(1024 * 1024), + baseRevisionId: z.string().uuid().nullable().optional(), clearLegacyPromptTemplate: z.boolean().optional().default(false), -}); +}).strict(); + +export const restoreAgentInstructionSchema = z.object({ + path: z.string().min(1).max(512), + revisionId: z.string().uuid(), + baseRevisionId: z.string().uuid(), +}).strict(); export type UpsertAgentInstructionsFile = z.infer; diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index 09c4fef7c4c..2b00f4cab09 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -984,3 +984,5 @@ export * from "./chat-channels.js"; export * from "./chat-github.js"; export * from "./email.js"; + +export { restoreAgentInstructionSchema } from "./agent.js"; diff --git a/server/src/__tests__/agent-instruction-revisions.test.ts b/server/src/__tests__/agent-instruction-revisions.test.ts new file mode 100644 index 00000000000..1af2f642e4e --- /dev/null +++ b/server/src/__tests__/agent-instruction-revisions.test.ts @@ -0,0 +1,240 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { randomUUID } from "node:crypto"; +import { beforeAll, afterAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { and, eq, sql } from "drizzle-orm"; +import { agents, agentApiKeys, companies, authUsers, companyMemberships, principalPermissionGrants, heartbeatRuns, + agentInstructionRevisions, agentInstructionHeads, issueThreadInteractions, issues, createDb } from "@paperclipai/db"; +import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; +import { agentInstructionRevisionService } from "../services/agent-instruction-revisions.js"; +import { instructionBytes, instructionPath, readInstructionBytes } from "../services/agent-instruction-files.js"; +import { agentInstructionsService, resolveManagedInstructionsRoot } from "../services/agent-instructions.js"; +import type { AuthorizationActor } from "../services/authorization.js"; +import { upsertAgentInstructionsFileSchema } from "@paperclipai/shared"; + +describe("canonical instruction revisions", () => { + let database: Awaited>; + let db: ReturnType; + let service: ReturnType; + const previousHome = process.env.PAPERCLIP_HOME; + let home: string; + let companyId: string, agentId: string, userId: string, actorId: string, runId: string, root: string; + let actor: AuthorizationActor; + const entryFile = "policy/INSTRUCTIONS.txt"; + const initial = "\uFEFF# Original\r\n☃\0\n"; + beforeAll(async () => { + home = await fs.mkdtemp(path.join(os.tmpdir(), "instruction-revisions-")); + process.env.PAPERCLIP_HOME = home; + database = await startEmbeddedPostgresTestDatabase("instruction-revisions-db-"); + db = createDb(database.connectionString); + service = agentInstructionRevisionService(db); + }, 90_000); + afterAll(async () => { + vi.restoreAllMocks(); + if (previousHome === undefined) delete process.env.PAPERCLIP_HOME; else process.env.PAPERCLIP_HOME = previousHome; + await database?.cleanup(); + if (home) await fs.rm(home, { recursive: true, force: true }); + }); + beforeEach(async () => { + companyId = randomUUID(); agentId = randomUUID(); actorId = randomUUID(); userId = randomUUID(); runId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Instruction tests", issuePrefix: randomUUID().slice(0, 8) }); + await db.insert(authUsers).values({ id: userId, name: "Editor", email: `${userId}@example.test`, createdAt: new Date(), updatedAt: new Date() }); + root = resolveManagedInstructionsRoot({ id: agentId, companyId, name: "Target", adapterConfig: {} }); + await db.insert(agents).values([ + { id: agentId, companyId, name: "Target", adapterConfig: { instructionsBundleMode: "managed", instructionsRootPath: root, instructionsEntryFile: entryFile } }, + { id: actorId, companyId, name: "Writer" }, + ]); + await db.insert(companyMemberships).values([ + { companyId, principalType: "user", principalId: userId, membershipRole: "operator" }, + { companyId, principalType: "agent", principalId: actorId, membershipRole: "member" }, + ]); + await db.insert(principalPermissionGrants).values({ companyId, principalType: "user", principalId: userId, permissionKey: "agents:configure", scope: { agentIds: [agentId] } }); + await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId: actorId, invocationSource: "on_demand", responsibleUserId: userId }); + actor = { type: "agent", companyId, agentId: actorId, runId, source: "agent_jwt" }; + await fs.mkdir(path.dirname(path.join(root, entryFile)), { recursive: true }); + await fs.writeFile(path.join(root, entryFile), initial); + }); + const target = () => ({ companyId, agentId }); + async function base() { return (await service.readCurrent(target(), actor))!; } + async function save(content: string, baseRevisionId: string | null) { + return service.commit({ ...target(), entryFile, content, baseRevisionId, source: "cleanup" }, actor); + } + it("seeds configured bytes, preserves immutable history, deduplicates, diffs and restores as a new revision", async () => { + const first = await base(); + expect(first.content).toBe(initial); + expect(first.revision).toMatchObject({ entryFile, source: "seed", responsibleUserId: userId, actorAgentId: actorId, sourceRunId: runId }); + const saved = await save("# Changed\r\n", first.revision.id); + expect(saved).toMatchObject({ changed: true, materialization: "current", revision: { parentRevisionId: first.revision.id, baseRevisionId: first.revision.id } }); + expect(await fs.readFile(path.join(root, entryFile), "utf8")).toBe(saved.content); + expect((await save(saved.content, first.revision.id)).revision.id).toBe(saved.revision.id); + const diff = await service.diff({ ...target(), entryFile, fromRevisionId: first.revision.id, toRevisionId: saved.revision.id }, actor); + expect(diff.prefix + diff.removed + diff.suffix).toBe(initial); + expect(diff.prefix + diff.added + diff.suffix).toBe(saved.content); + const restored = await service.restore({ ...target(), entryFile, revisionId: first.revision.id, baseRevisionId: saved.revision.id }, actor); + expect(restored.content).toBe(initial); + expect(restored.revision).toMatchObject({ source: "restore", parentRevisionId: saved.revision.id, restoredFromRevisionId: first.revision.id }); + expect(restored.revision.id).not.toBe(first.revision.id); + expect((await service.history({ ...target(), entryFile }, actor)).revisions).toHaveLength(3); + const page = await service.history({ ...target(), entryFile, limit: 1 }, actor); + expect((await service.history({ ...target(), entryFile, cursor: page.nextCursor!, limit: 1 }, actor)).revisions[0].id).toBe(saved.revision.id); + }); + it("does not expose a rolled-back seed revision when a writer omitted the initial read", async () => { + await expect(save("overwrite without reading", null)).rejects.toMatchObject({ status: 409, details: { currentRevisionId: null } }); + expect(await db.select().from(agentInstructionRevisions).where(eq(agentInstructionRevisions.agentId, agentId))).toHaveLength(0); + expect((await base()).content).toBe(initial); + }); + it("paginates without dropping revisions within one fractional millisecond", async () => { + const first = await base(); + const second = await save("second", first.revision.id); + const third = await save("third", second.revision.id); + // Give this fixture timestamps that JS Date cannot represent exactly. + for (const [id, time] of [[first.revision.id, "2026-01-01 00:00:00.123100+00"], [second.revision.id, "2026-01-01 00:00:00.123200+00"], [third.revision.id, "2026-01-01 00:00:00.123300+00"]]) { + await db.execute(sql`update agent_instruction_revisions set created_at = ${time}::timestamptz where id = ${id}`); + } + const a = await service.history({ ...target(), entryFile, limit: 1 }, actor); + const b = await service.history({ ...target(), entryFile, limit: 1, cursor: a.nextCursor! }, actor); + const c = await service.history({ ...target(), entryFile, limit: 1, cursor: b.nextCursor! }, actor); + expect([a, b, c].map((page) => page.revisions[0].id)).toEqual([third.revision.id, second.revision.id, first.revision.id]); + expect(c.nextCursor).toBeNull(); + }); + it("serializes competing saves and refuses stale cleanup after a restore or board edit", async () => { + const first = await base(); + const results = await Promise.allSettled([save("one", first.revision.id), save("two", first.revision.id)]); + expect(results.filter((r) => r.status === "fulfilled")).toHaveLength(1); + expect(results.find((r) => r.status === "rejected")).toMatchObject({ reason: { status: 409, details: { code: "INSTRUCTION_REVISION_CONFLICT" } } }); + expect((await service.history({ ...target(), entryFile }, actor)).revisions).toHaveLength(2); + const current = await base(); + await service.restore({ ...target(), entryFile, baseRevisionId: current.revision.id, revisionId: first.revision.id }, actor); + await expect(save("stale cleanup", current.revision.id)).rejects.toMatchObject({ status: 409 }); + }); + it("commits before materializing and recovers the committed head after a new service instance", async () => { + const first = await base(); + const rename = vi.spyOn(fs, "rename").mockRejectedValueOnce(new Error("simulated interrupted materialization")); + const saved = await save("durable", first.revision.id); + rename.mockRestore(); + expect(saved.materialization).toBe("pending"); + expect(await fs.readFile(path.join(root, entryFile), "utf8")).toBe(initial); + service = agentInstructionRevisionService(db); + expect((await base()).content).toBe("durable"); + await service.materializeCurrent(target()); + expect(await fs.readFile(path.join(root, entryFile), "utf8")).toBe("durable"); + await fs.unlink(path.join(root, entryFile)); + await service.materializeCurrent(target()); + expect(await fs.readFile(path.join(root, entryFile), "utf8")).toBe("durable"); + }); + it("rolls back revision and head together when the database rejects head movement", async () => { + const first = await base(); + await db.execute(sql`CREATE FUNCTION reject_instruction_head_test() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RAISE EXCEPTION 'injected head failure'; END $$`); + await db.execute(sql`CREATE TRIGGER reject_instruction_head_test BEFORE UPDATE ON agent_instruction_heads FOR EACH ROW EXECUTE FUNCTION reject_instruction_head_test()`); + try { await expect(save("lost", first.revision.id)).rejects.toThrow(); } + finally { await db.execute(sql`DROP TRIGGER reject_instruction_head_test ON agent_instruction_heads`); await db.execute(sql`DROP FUNCTION reject_instruction_head_test()`); } + expect((await base()).revision.id).toBe(first.revision.id); + expect((await service.history({ ...target(), entryFile }, actor)).revisions).toHaveLength(1); + }); + it("allows ordinary peer edits without agent-admin but rejects missing, forged and revoked responsible identity", async () => { + const first = await base(); + await save("peer save", first.revision.id); + await expect(service.commit({ ...target(), entryFile, content: "forged", baseRevisionId: first.revision.id, source: "tool" }, { ...actor, onBehalfOfUserId: "forged" })).rejects.toMatchObject({ status: 403 }); + await db.update(heartbeatRuns).set({ responsibleUserId: null }).where(eq(heartbeatRuns.id, runId)); + await expect(save("missing", first.revision.id)).rejects.toMatchObject({ status: 403 }); + await db.update(heartbeatRuns).set({ responsibleUserId: userId }).where(eq(heartbeatRuns.id, runId)); + await db.update(companyMemberships).set({ status: "suspended" }).where(and(eq(companyMemberships.principalId, userId), eq(companyMemberships.companyId, companyId))); + await expect(save("revoked", first.revision.id)).rejects.toMatchObject({ status: 403 }); + }); + it("rechecks user target scope and agent containment, even with shadow authorization enabled", async () => { + const first = await base(); + const previousShadow = process.env.PAPERCLIP_RESPONSIBLE_USER_AUTHZ_SHADOW; + process.env.PAPERCLIP_RESPONSIBLE_USER_AUTHZ_SHADOW = "true"; + await db.update(principalPermissionGrants).set({ scope: { agentIds: [actorId] } }).where(eq(principalPermissionGrants.principalId, userId)); + await expect(save("denied", first.revision.id)).rejects.toMatchObject({ status: 403 }); + await db.update(principalPermissionGrants).set({ scope: null }).where(eq(principalPermissionGrants.principalId, userId)); + await db.update(agents).set({ permissions: { trustPreset: "low_trust_review" } }).where(eq(agents.id, actorId)); + await expect(save("restricted", first.revision.id)).rejects.toMatchObject({ status: 403 }); + if (previousShadow === undefined) delete process.env.PAPERCLIP_RESPONSIBLE_USER_AUTHZ_SHADOW; else process.env.PAPERCLIP_RESPONSIBLE_USER_AUTHZ_SHADOW = previousShadow; + }); + it("keeps suggest-only protected-change consent and explicit configure scope restrictions", async () => { + const first = await base(); + await db.insert(principalPermissionGrants).values({ companyId, principalType: "agent", principalId: actorId, permissionKey: "agents:suggest-changes" }); + await expect(save("needs approval", first.revision.id)).rejects.toMatchObject({ status: 403 }); + await db.delete(principalPermissionGrants).where(eq(principalPermissionGrants.principalId, actorId)); + await db.insert(principalPermissionGrants).values({ companyId, principalType: "agent", principalId: actorId, permissionKey: "agents:configure", scope: { agentIds: [actorId] } }); + await expect(save("out of scope", first.revision.id)).rejects.toMatchObject({ status: 403 }); + }); + it("consumes accepted protected-change approval only with a successful CAS commit", async () => { + const first = await base(); + const sourceRunId = randomUUID(), issueId = randomUUID(), interactionId = randomUUID(); + await db.insert(heartbeatRuns).values({ id: sourceRunId, companyId, agentId: actorId, status: "succeeded" }); + await db.insert(issues).values({ id: issueId, companyId, title: "Review instructions" }); + await db.insert(principalPermissionGrants).values({ companyId, principalType: "agent", principalId: actorId, permissionKey: "agents:suggest-changes" }); + await db.insert(issueThreadInteractions).values({ id: interactionId, companyId, issueId, kind: "request_confirmation", status: "accepted", sourceRunId, createdByAgentId: actorId, + payload: { version: 1, prompt: "Apply the instruction change?", detailsMarkdown: "```diff\n+approved\n```", target: { type: "custom", key: `agent:${agentId}:instructions`, revisionId: "proposal" } }, + result: { version: 1, outcome: "accepted" }, resolvedByUserId: userId, resolvedAt: new Date(), + }); + await expect(save("approved", randomUUID())).rejects.toMatchObject({ status: 409 }); + expect((await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, interactionId)))[0].result).not.toHaveProperty("consumedAt"); + const receipt = await save("approved", first.revision.id); + expect(receipt.changed).toBe(true); + expect((await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, interactionId)))[0].result).toMatchObject({ consumedByRunId: runId }); + await expect(save("another unapproved change", receipt.revision.id)).rejects.toMatchObject({ status: 403 }); + }); + it("shares head/CAS and attribution between board and agent writers", async () => { + const first = await base(); + const board = { type: "board", userId, source: "session" } as const; + const saved = await service.commit({ ...target(), entryFile, content: "board edit", baseRevisionId: first.revision.id, source: "board" }, board); + expect(saved.revision).toMatchObject({ actorUserId: userId, responsibleUserId: userId, actorAgentId: null, sourceRunId: null, source: "board" }); + await expect(save("stale agent edit", first.revision.id)).rejects.toMatchObject({ status: 409 }); + }); + it("seeds a nested legacy managed entry and never imports a different AGENTS.md", async () => { + await fs.writeFile(path.join(root, "AGENTS.md"), "unrelated"); + await db.update(agents).set({ adapterConfig: { instructionsFilePath: path.join(root, entryFile) } }).where(eq(agents.id, agentId)); + const first = await base(); + expect(first.content).toBe(initial); + expect(first.revision.entryFile).toBe(entryFile); + await db.update(agents).set({ adapterConfig: { instructionsBundleMode: "managed", instructionsRootPath: root, instructionsEntryFile: "missing.md" } }).where(eq(agents.id, agentId)); + expect(await service.readCurrent(target(), actor)).toBeNull(); + await expect(save("wrong entry", first.revision.id)).rejects.toMatchObject({ status: 409, details: { code: "INSTRUCTION_ENTRY_CHANGED" } }); + }); + it("rejects cross-company targets, foreign revisions, and revoked API keys", async () => { + const first = await base(); + await expect(service.readRevision({ ...target(), entryFile, revisionId: randomUUID() }, actor)).rejects.toMatchObject({ status: 404 }); + await expect(service.readCurrent({ companyId: randomUUID(), agentId }, actor)).rejects.toMatchObject({ status: 404 }); + const keyId = randomUUID(); + await db.insert(agentApiKeys).values({ id: keyId, companyId, agentId: actorId, name: "test", keyHash: randomUUID(), responsibleUserId: userId, revokedAt: new Date() }); + await expect(service.commit({ ...target(), entryFile, content: "key", baseRevisionId: first.revision.id, source: "api" }, { ...actor, keyId })).rejects.toMatchObject({ status: 403 }); + await expect(db.insert(agentInstructionHeads).values({ companyId: randomUUID(), agentId, entryFile, revisionId: first.revision.id })).rejects.toThrow(); + }); + it("rejects invalid content, symlinks, traversal and external bundles without changing canonical bytes", async () => { + const first = await base(); + expect(() => instructionBytes("\ud800")).toThrow("UTF-8"); + expect(() => instructionBytes(new Uint8Array([0xff]))).toThrow("UTF-8"); + expect(() => instructionBytes("a".repeat(1024 * 1024 + 1))).toThrow("1 MiB"); + for (const value of ["../AGENTS.md", "/etc/passwd", "a/../AGENTS.md", "a\\b", "a//b"]) expect(() => instructionPath(value)).toThrow(); + await fs.unlink(path.join(root, entryFile)); + await fs.symlink(path.join(home, "outside"), path.join(root, entryFile)); + await expect(save("symlink", first.revision.id)).rejects.toMatchObject({ status: 422 }); + await fs.unlink(path.join(root, entryFile)); + await fs.rm(path.dirname(path.join(root, entryFile)), { recursive: true }); + await fs.symlink(home, path.dirname(path.join(root, entryFile))); + await expect(readInstructionBytes(root, entryFile)).rejects.toMatchObject({ status: 422 }); + await db.update(agents).set({ adapterConfig: { instructionsBundleMode: "external", instructionsRootPath: home } }).where(eq(agents.id, agentId)); + await expect(save("external", first.revision.id)).rejects.toMatchObject({ details: { code: "INSTRUCTION_MANAGED_BUNDLE_REQUIRED" } }); + expect((await db.select().from(agentInstructionRevisions).where(eq(agentInstructionRevisions.agentId, agentId)))).toHaveLength(1); + }); + it("does not overwrite history through bundle replacement or unversioned entry writers", async () => { + await base(); + const [agent] = await db.select().from(agents).where(eq(agents.id, agentId)); + await expect(agentInstructionsService(db).writeFile(agent, entryFile, "bypass")).rejects.toMatchObject({ details: { code: "INSTRUCTION_REVISION_REQUIRED" } }); + await expect(agentInstructionsService(db).materializeManagedBundle(agent, { [entryFile]: "bypass" }, { entryFile, replaceExisting: true })).rejects.toMatchObject({ details: { code: "INSTRUCTION_REVISION_REQUIRED" } }); + const files = agentInstructionsService(db); + await db.update(agents).set({ adapterConfig: { ...agent.adapterConfig, instructionsEntryFile: "NEXT.md", instructionsFilePath: path.join(root, "NEXT.md") } }).where(eq(agents.id, agentId)); + await fs.writeFile(path.join(root, "NEXT.md"), "new configured entry"); + // A route's stale agent snapshot must not classify the new entry as supporting content. + await expect(files.writeFile(agent, "NEXT.md", "bypass")).rejects.toMatchObject({ details: { code: "INSTRUCTION_REVISION_REQUIRED" } }); + await expect(files.deleteFile(agent, "NEXT.md")).rejects.toMatchObject({ status: 422 }); + await expect(files.writeFile(agent, entryFile, "historical bypass")).rejects.toMatchObject({ details: { code: "INSTRUCTION_REVISION_REQUIRED" } }); + await expect(files.materializeManagedBundle(agent, { "NEXT.md": "new configured entry" }, { entryFile: "NEXT.md", replaceExisting: true })).rejects.toMatchObject({ details: { code: "INSTRUCTION_REVISION_REQUIRED" } }); + expect(await fs.readFile(path.join(root, "NEXT.md"), "utf8")).toBe("new configured entry"); + expect(upsertAgentInstructionsFileSchema.safeParse({ path: entryFile, content: "bad", responsibleUserId: userId }).success).toBe(false); + }); +}); diff --git a/server/src/__tests__/agent-instructions-routes.test.ts b/server/src/__tests__/agent-instructions-routes.test.ts index 0a110b78025..5fdfc494d95 100644 --- a/server/src/__tests__/agent-instructions-routes.test.ts +++ b/server/src/__tests__/agent-instructions-routes.test.ts @@ -24,6 +24,9 @@ const mockAgentInstructionsService = vi.hoisted(() => ({ materializeManagedBundle: vi.fn(), })); +const mockInstructionRevisions = vi.hoisted(() => ({ readCurrent: vi.fn(), commit: vi.fn(), restore: vi.fn(), history: vi.fn(), readRevision: vi.fn(), diff: vi.fn(), materializeCurrent: vi.fn() })); +vi.mock("../services/agent-instruction-revisions.js", () => ({ agentInstructionRevisionService: () => mockInstructionRevisions })); + const mockAccessService = vi.hoisted(() => ({ canUser: vi.fn(), decide: vi.fn(), @@ -202,6 +205,8 @@ describe("agent instructions bundle routes", () => { vi.doUnmock("../middleware/index.js"); registerModuleMocks(); vi.clearAllMocks(); + mockInstructionRevisions.readCurrent.mockResolvedValue(null); + mockInstructionRevisions.commit.mockResolvedValue({ revision: { id: "33333333-3333-4333-8333-333333333333", entryFile: "AGENTS.md", byteLength: 18 }, content: "# Updated Agent\n", changed: true, materialization: "current" }); mockBuiltInAgentService.ensureCompanyDefaultAgentGrants.mockResolvedValue(0); mockSyncInstructionsBundleConfigFromFilePath.mockImplementation((_agent, config) => config); mockFindServerAdapter.mockImplementation((_type: string) => ({ type: _type })); @@ -484,7 +489,7 @@ describe("agent instructions bundle routes", () => { expect(mockAgentService.create).not.toHaveBeenCalled(); }); - it("denies non-privileged agents from reading peer instructions bundles", async () => { + it("denies instruction reads when the visibility policy denies access", async () => { mockAgentService.getById.mockImplementation(async (id: string) => { if (id === "agent-reader") { return { @@ -516,7 +521,7 @@ describe("agent instructions bundle routes", () => { expect(res.status, JSON.stringify(res.body)).toBe(403); expect(res.body.error).toContain("Missing permission"); expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ - action: "agent_config:read", + action: "agent:read", resource: { type: "agent", companyId: "company-1", @@ -575,7 +580,7 @@ describe("agent instructions bundle routes", () => { expect(res.status, JSON.stringify(res.body)).toBe(200); expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ - action: "agent_config:read", + action: "agent:read", resource: { type: "agent", companyId: "company-1", @@ -588,34 +593,52 @@ describe("agent instructions bundle routes", () => { ); }); - it("writes a bundle file and persists compatibility config", async () => { + it("commits entry bytes through the canonical service with the authenticated actor", async () => { const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) .put("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle/file?companyId=company-1") - .send({ - path: "AGENTS.md", - content: "# Updated Agent\n", - clearLegacyPromptTemplate: true, - })); - + .send({ path: "AGENTS.md", content: "# Updated Agent\n", baseRevisionId: null })); expect(res.status, JSON.stringify(res.body)).toBe(200); - expect(mockAgentInstructionsService.writeFile).toHaveBeenCalledWith( - expect.objectContaining({ id: "11111111-1111-4111-8111-111111111111" }), - "AGENTS.md", - "# Updated Agent\n", - { clearLegacyPromptTemplate: true }, - ); - expect(mockAgentService.update).toHaveBeenCalledWith( - "11111111-1111-4111-8111-111111111111", - expect.objectContaining({ - adapterConfig: expect.objectContaining({ - instructionsBundleMode: "managed", - instructionsRootPath: "/tmp/agent-1", - instructionsEntryFile: "AGENTS.md", - instructionsFilePath: "/tmp/agent-1/AGENTS.md", - }), - }), - expect.any(Object), - ); + expect(mockInstructionRevisions.commit).toHaveBeenCalledWith(expect.objectContaining({ + agentId: "11111111-1111-4111-8111-111111111111", entryFile: "AGENTS.md", content: "# Updated Agent\n", baseRevisionId: null, source: "board", + }), expect.objectContaining({ type: "board" })); + expect(mockAgentInstructionsService.writeFile).not.toHaveBeenCalled(); + expect(res.body.receipt.revision.id).toBe("33333333-3333-4333-8333-333333333333"); + }); + + it("returns committed entry content independently of its disk copy", async () => { + mockInstructionRevisions.readCurrent.mockResolvedValue({ revision: { id: "33333333-3333-4333-8333-333333333333", entryFile: "AGENTS.md", byteLength: 9 }, content: "committed" }); + const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) + .get("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle/file?path=AGENTS.md")); + expect(res.status).toBe(200); + expect(res.body.content).toBe("committed"); + expect(res.body.revision.id).toBe("33333333-3333-4333-8333-333333333333"); + expect(mockAgentInstructionsService.readFile).not.toHaveBeenCalled(); + }); + + it("exposes scoped history, diff and restore with the server actor", async () => { + const app = await createApp(); + const revisionId = "33333333-3333-4333-8333-333333333333"; + const baseRevisionId = "44444444-4444-4444-8444-444444444444"; + const prefix = "/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle"; + mockInstructionRevisions.history.mockResolvedValue({ revisions: [], nextCursor: null }); + mockInstructionRevisions.diff.mockResolvedValue({ removed: "old", added: "new" }); + mockInstructionRevisions.restore.mockResolvedValue({ revision: { id: revisionId, entryFile: "AGENTS.md", byteLength: 3 }, content: "old", changed: true, materialization: "current" }); + expect((await requestApp(app, (url) => request(url).get(`${prefix}/history?path=AGENTS.md`))).status).toBe(200); + expect((await requestApp(app, (url) => request(url).get(`${prefix}/diff?path=AGENTS.md&from=${revisionId}&to=${baseRevisionId}`))).status).toBe(200); + const restored = await requestApp(app, (url) => request(url).post(`${prefix}/restore`).send({ path: "AGENTS.md", revisionId, baseRevisionId })); + expect(restored.status).toBe(200); + expect(restored.body.receipt.changed).toBe(true); + expect(mockInstructionRevisions.restore).toHaveBeenCalledWith({ companyId: "company-1", agentId: "11111111-1111-4111-8111-111111111111", entryFile: "AGENTS.md", revisionId, baseRevisionId }, expect.objectContaining({ type: "board", userId: "local-board" })); + }); + + it("requires a base revision and rejects client-supplied responsible identity", async () => { + const app = await createApp(); + for (const body of [ { path: "AGENTS.md", content: "stale" }, { path: "AGENTS.md", content: "forged", baseRevisionId: null, responsibleUserId: "forged" } ]) { + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .put("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle/file").send(body)); + expect([400, 422]).toContain(res.status); + } + expect(mockInstructionRevisions.commit).not.toHaveBeenCalled(); }); it("preserves managed instructions config when switching adapters", async () => { diff --git a/server/src/__tests__/agent-instructions-service.test.ts b/server/src/__tests__/agent-instructions-service.test.ts index 2dc4719d1a8..948e55d47e7 100644 --- a/server/src/__tests__/agent-instructions-service.test.ts +++ b/server/src/__tests__/agent-instructions-service.test.ts @@ -53,7 +53,7 @@ describe("agent instructions service", () => { await fs.mkdir(path.join(externalRoot, "docs"), { recursive: true }); await fs.writeFile(path.join(externalRoot, "docs", "TOOLS.md"), "## Tools\n", "utf8"); - const svc = agentInstructionsService(); + const svc = agentInstructionsService({ select: () => ({ from: () => ({ where: async () => [] }) }) } as never); const agent = makeAgent({ instructionsBundleMode: "external", instructionsRootPath: externalRoot, @@ -250,11 +250,10 @@ describe("agent instructions service", () => { expect(bundle.mode).toBe("managed"); expect(bundle.rootPath).toBe(managedRoot); - expect(bundle.entryFile).toBe("AGENTS.md"); + expect(bundle.entryFile).toBe("docs/MISSING.md"); expect(bundle.files.map((file) => file.path)).toEqual(["AGENTS.md"]); expect(bundle.warnings).toEqual([ `Recovered managed instructions from disk at ${managedRoot}; ignoring stale configured root ${staleRoot}.`, - "Recovered managed instructions entry file from disk as AGENTS.md; previous entry docs/MISSING.md was missing.", ]); expect(exported.files).toEqual({ "AGENTS.md": "# Managed Agent\n" }); }); @@ -293,8 +292,8 @@ describe("agent instructions service", () => { expect(result.adapterConfig).toMatchObject({ instructionsBundleMode: "managed", instructionsRootPath: managedRoot, - instructionsEntryFile: "AGENTS.md", - instructionsFilePath: path.join(managedRoot, "AGENTS.md"), + instructionsEntryFile: "docs/MISSING.md", + instructionsFilePath: path.join(managedRoot, "docs/MISSING.md"), }); await expect(fs.readFile(path.join(managedRoot, "docs", "TOOLS.md"), "utf8")).resolves.toBe("## Tools\n"); }); @@ -334,8 +333,8 @@ describe("agent instructions service", () => { expect(result.adapterConfig).toMatchObject({ instructionsBundleMode: "managed", instructionsRootPath: managedRoot, - instructionsEntryFile: "AGENTS.md", - instructionsFilePath: path.join(managedRoot, "AGENTS.md"), + instructionsEntryFile: "docs/MISSING.md", + instructionsFilePath: path.join(managedRoot, "docs/MISSING.md"), }); await expect(fs.stat(path.join(managedRoot, "docs", "TOOLS.md"))).rejects.toThrow(); expect(result.bundle.files.map((file) => file.path)).toEqual(["AGENTS.md"]); @@ -373,11 +372,10 @@ describe("agent instructions service", () => { expect(bundle.mode).toBe("managed"); expect(bundle.rootPath).toBe(managedRoot); - expect(bundle.entryFile).toBe("AGENTS.md"); + expect(bundle.entryFile).toBe("docs/MISSING.md"); expect(bundle.files.map((file) => file.path)).toEqual(["AGENTS.md"]); expect(bundle.warnings).toEqual([ `Recovered managed instructions from disk at ${managedRoot}; ignoring stale configured root ${staleRoot}.`, - "Recovered managed instructions entry file from disk as AGENTS.md; previous entry docs/MISSING.md was missing.", ]); expect(exported.files).toEqual({ "AGENTS.md": "# Managed Agent\n" }); }); diff --git a/server/src/__tests__/built-in-agent-routes.test.ts b/server/src/__tests__/built-in-agent-routes.test.ts index acae6704dc3..e51166f2ba4 100644 --- a/server/src/__tests__/built-in-agent-routes.test.ts +++ b/server/src/__tests__/built-in-agent-routes.test.ts @@ -412,7 +412,7 @@ describe("built-in agent routes", () => { .send({}); expect(res.status, JSON.stringify(res.body)).toBe(200); - expect(mockBuiltInAgentService.reset).toHaveBeenCalledWith(companyId, "briefs", {}); + expect(mockBuiltInAgentService.reset).toHaveBeenCalledWith(companyId, "briefs", {}, expect.objectContaining({ type: "agent", agentId: "manager-agent" })); expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ companyId, actorType: "agent", diff --git a/server/src/__tests__/built-in-agents.test.ts b/server/src/__tests__/built-in-agents.test.ts index 8f3abf3972d..36f4a349e6b 100644 --- a/server/src/__tests__/built-in-agents.test.ts +++ b/server/src/__tests__/built-in-agents.test.ts @@ -27,6 +27,7 @@ import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase, } from "./helpers/embedded-postgres.js"; +import { agentInstructionRevisionService } from "../services/agent-instruction-revisions.js"; import { agentInstructionsService } from "../services/agent-instructions.ts"; import { agentService } from "../services/agents.ts"; import { approvalService } from "../services/approvals.ts"; @@ -116,6 +117,16 @@ describe("built-in agent asset loading", () => { describeEmbeddedPostgres("built-in agents", () => { let db!: ReturnType; let tempDb: Awaited> | null = null; + const instructionOperator = { type: "board", userId: "local-board", source: "local_implicit" } as const; + async function writeInstructionEntry(agent: { id: string; companyId: string }, entryFile: string, content: string) { + const service = agentInstructionRevisionService(db); + const target = { companyId: agent.companyId, agentId: agent.id }; + const baseline = await service.readCurrent(target, instructionOperator); + await service.commit({ ...target, entryFile, content, baseRevisionId: baseline?.revision.id ?? null, source: "board" }, instructionOperator); + const refreshed = await agentService(db).getById(agent.id); + return { adapterConfig: refreshed!.adapterConfig }; + } + beforeAll(async () => { tempDb = await startEmbeddedPostgresTestDatabase("paperclip-built-in-agents-"); @@ -553,7 +564,7 @@ describeEmbeddedPostgres("built-in agents", () => { capabilities: "Custom purpose", }); - const reset = await builtIns.reset(companyId, "briefs"); + const reset = await builtIns.reset(companyId, "briefs", {}, instructionOperator); expect(reset).toMatchObject({ status: "ready", @@ -801,7 +812,7 @@ describeEmbeddedPostgres("built-in agents", () => { const created = await builtIns.ensure(companyId, "reflection-coach"); const instructions = agentInstructionsService(); - await instructions.writeFile(created.agent!, "AGENTS.md", "# Custom Reflection Coach\n\nOperator edit.\n"); + await writeInstructionEntry(created.agent!, "AGENTS.md", "# Custom Reflection Coach\n\nOperator edit.\n"); const reconciled = await builtIns.ensure(companyId, "reflection-coach"); const drift = reconciled.resources.find((resource) => resource.resourceKind === "instructions"); @@ -815,7 +826,7 @@ describeEmbeddedPostgres("built-in agents", () => { content: "# Custom Reflection Coach\n\nOperator edit.\n", }); - const reset = await builtIns.reset(companyId, "reflection-coach"); + const reset = await builtIns.reset(companyId, "reflection-coach", {}, instructionOperator); expect(reset.resources.find((resource) => resource.resourceKind === "instructions")).toMatchObject({ stockStatus: "stock_current", resetAvailable: false, @@ -823,6 +834,13 @@ describeEmbeddedPostgres("built-in agents", () => { const resetFile = await instructions.readFile(reset.agent!, "AGENTS.md"); expect(resetFile.content).toContain("Reflection Coach"); expect(resetFile.content).not.toContain("Operator edit."); + const revisions = agentInstructionRevisionService(db); + const target = { companyId, agentId: created.agent!.id, entryFile: "AGENTS.md" }; + const history = await revisions.history(target, instructionOperator); + expect(history.revisions).toHaveLength(3); + expect(history.revisions[0]).toMatchObject({ source: "board", actorUserId: "local-board" }); + const previous = await revisions.readRevision({ ...target, revisionId: history.revisions[1]!.id }, instructionOperator); + expect(previous.content).toBe("# Custom Reflection Coach\n\nOperator edit.\n"); }); it("blocks deleting a built-in agent", async () => { @@ -1270,7 +1288,7 @@ describeEmbeddedPostgres("built-in agents", () => { const created = await builtIns.ensure(companyId, "summarizer"); const instructions = agentInstructionsService(); - await instructions.writeFile(created.agent!, "AGENTS.md", "# Custom Summarizer\n\nOperator edit.\n"); + await writeInstructionEntry(created.agent!, "AGENTS.md", "# Custom Summarizer\n\nOperator edit.\n"); const reconciled = await builtIns.ensure(companyId, "summarizer"); expect(reconciled.resources.find((resource) => resource.resourceKind === "instructions")).toMatchObject({ @@ -1279,7 +1297,7 @@ describeEmbeddedPostgres("built-in agents", () => { changedFiles: ["AGENTS.md"], }); - const reset = await builtIns.reset(companyId, "summarizer"); + const reset = await builtIns.reset(companyId, "summarizer", {}, instructionOperator); expect(reset.resources.find((resource) => resource.resourceKind === "instructions")).toMatchObject({ stockStatus: "stock_current", resetAvailable: false, @@ -1419,7 +1437,7 @@ describeEmbeddedPostgres("built-in agents", () => { const coach = created.agent!; const instructionsSvc = agentInstructionsService(); const originalInstructions = "# Target Coder\n\nWork from the assigned issue.\n"; - const prepared = await instructionsSvc.writeFile(target, "AGENTS.md", originalInstructions); + const prepared = await writeInstructionEntry(target, "AGENTS.md", originalInstructions); let persistedTarget = (await agentsSvc.update(target.id, { adapterConfig: prepared.adapterConfig }))!; const interactionsSvc = issueThreadInteractionService(db); @@ -1431,7 +1449,7 @@ describeEmbeddedPostgres("built-in agents", () => { if (interaction?.kind !== "request_confirmation" || interaction.status !== "accepted") { return false; } - const written = await instructionsSvc.writeFile(persistedTarget, "AGENTS.md", input.nextInstructions); + const written = await writeInstructionEntry(persistedTarget, "AGENTS.md", input.nextInstructions); persistedTarget = (await agentsSvc.update(persistedTarget.id, { adapterConfig: written.adapterConfig }))!; return true; }; @@ -1568,7 +1586,7 @@ describeEmbeddedPostgres("built-in agents", () => { const agent = created.agent!; const instructionsSvc = agentInstructionsService(); - await instructionsSvc.writeFile(agent, "AGENTS.md", "# Custom Reflection Coach\n\nDo not overwrite me.\n"); + await writeInstructionEntry(agent, "AGENTS.md", "# Custom Reflection Coach\n\nDo not overwrite me.\n"); await db .update(companySkills) .set({ markdown: "---\nname: reflection-coach\n---\n\n# Custom skill\n" }) @@ -1597,7 +1615,7 @@ describeEmbeddedPostgres("built-in agents", () => { const reset = await builtInAgentService(db).reset(companyId, "reflection-coach", { resources: ["instructions", "routine"], - }); + }, instructionOperator); expect(reset.resources.find((resource) => resource.resourceKind === "instructions")).toMatchObject({ stockStatus: "stock_current", resetAvailable: false, diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 2efcc764571..1d29fa758ad 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -38,6 +38,7 @@ import { type AgentSkillSnapshot, type InstanceSchedulerHeartbeatAgent, upsertAgentInstructionsFileSchema, + restoreAgentInstructionSchema, updateAgentInstructionsBundleSchema, updateAgentPermissionsSchema, updateAgentInstructionsPathSchema, @@ -63,7 +64,9 @@ import { import { trackAgentCreated } from "@paperclipai/shared/telemetry"; import { validate } from "../middleware/validate.js"; import { inheritNativeRunnerAdapterConfig } from "../services/native-runtime/native-agent-runtime-inheritance.js"; -import { agentInstructionsBundleMode } from "../services/agent-instructions.js"; +import { agentInstructionRevisionService } from "../services/agent-instruction-revisions.js"; +import { instructionPath } from "../services/agent-instruction-files.js"; +import { agentInstructionsBundleMode, deriveBundleState } from "../services/agent-instructions.js"; import { agentService, agentInstructionsService, @@ -724,7 +727,15 @@ export function agentRoutes( const recovery = recoveryService(db, { enqueueWakeup: heartbeat.wakeup }); const issueApprovalsSvc = issueApprovalService(db); const secretsSvc = secretService(db); - const instructions = agentInstructionsService(); + const instructions = agentInstructionsService(db); + const instructionRevisions = agentInstructionRevisionService(db); + function instructionFileDetail(snapshot: import("@paperclipai/shared").AgentInstructionSnapshot, + receipt?: import("@paperclipai/shared").AgentInstructionCommitReceipt) { + const path = snapshot.revision.entryFile; + return { path, content: snapshot.content, size: snapshot.revision.byteLength, revision: snapshot.revision, receipt, + language: path.toLowerCase().endsWith(".md") ? "markdown" : "text", markdown: path.toLowerCase().endsWith(".md"), + isEntryFile: true, editable: true, deprecated: false, virtual: false }; + } const companySkills = companySkillService(db); const workspaceOperations = workspaceOperationService(db); const instanceSettings = instanceSettingsService(db); @@ -5067,8 +5078,21 @@ export function agentRoutes( const id = req.params.id as string; const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); if (!existing) return; - await assertCanReadAgent(req, existing); + const readDecision = await access.decide({ actor: req.actor, action: "agent:read", resource: { type: "agent", companyId: existing.companyId, agentId: existing.id } }); + if (!readDecision.allowed) throw forbidden(readDecision.explanation, authorizationDeniedDetails(readDecision)); assertExternalInstructionsAdmin(req, existing); + if (agentInstructionsBundleMode(existing) !== "external") { + const target = { companyId: existing.companyId, agentId: existing.id }; + const current = await instructionRevisions.readCurrent(target, req.actor); + if (current) { + try { await instructionRevisions.materializeCurrent(target); } + catch (error) { + const bundle = await instructions.getBundle(existing); + bundle.warnings.push(`Saved instruction revision needs materialization: ${error instanceof Error ? error.message : String(error)}`); + res.json(bundle); return; + } + } + } res.json(await instructions.getBundle(existing)); }); @@ -5124,7 +5148,8 @@ export function agentRoutes( const id = req.params.id as string; const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); if (!existing) return; - await assertCanReadAgent(req, existing); + const readDecision = await access.decide({ actor: req.actor, action: "agent:read", resource: { type: "agent", companyId: existing.companyId, agentId: existing.id } }); + if (!readDecision.allowed) throw forbidden(readDecision.explanation, authorizationDeniedDetails(readDecision)); assertExternalInstructionsAdmin(req, existing); const relativePath = typeof req.query.path === "string" ? req.query.path : ""; @@ -5133,6 +5158,10 @@ export function agentRoutes( return; } + if (agentInstructionsBundleMode(existing) !== "external" && instructionPath(relativePath) === deriveBundleState(existing).entryFile) { + const snapshot = await instructionRevisions.readCurrent({ companyId: existing.companyId, agentId: existing.id }, req.actor); + if (snapshot) { res.json(instructionFileDetail(snapshot)); return; } + } res.json(await instructions.readFile(existing, relativePath)); }); @@ -5140,6 +5169,27 @@ export function agentRoutes( const id = req.params.id as string; const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); if (!existing) return; + const entryFile = deriveBundleState(existing).entryFile; + if (instructionPath(req.body.path) === entryFile) { + assertExternalInstructionsAdmin(req, existing); + if (req.body.baseRevisionId === undefined) throw unprocessable("Read the entry and supply baseRevisionId (null for a new entry)", { code: "INSTRUCTION_BASE_REQUIRED" }); + // Clearing legacy prompt configuration remains a protected config change. + if (req.body.clearLegacyPromptTemplate) await assertCanManageInstructionsPath(req, existing); + const receipt = await instructionRevisions.commit({ companyId: existing.companyId, agentId: existing.id, + entryFile, content: req.body.content, baseRevisionId: req.body.baseRevisionId, + source: req.actor.type === "board" ? "board" : "api" }, req.actor); + if (req.body.clearLegacyPromptTemplate) { + const fresh = await svc.getById(existing.id); + if (fresh) { + const adapterConfig = { ...asRecord(fresh.adapterConfig) }; + delete adapterConfig.promptTemplate; + delete adapterConfig.bootstrapPromptTemplate; + await svc.update(existing.id, { adapterConfig }); + } + } + res.json(instructionFileDetail(receipt, receipt)); + return; + } await assertCanManageInstructionsPath(req, existing); assertExternalInstructionsAdmin(req, existing); @@ -5184,6 +5234,43 @@ export function agentRoutes( res.json(result.file); }); + router.get("/agents/:id/instructions-bundle/history", async (req, res) => { + const existing = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Agent not found"); + if (!existing) return; + assertExternalInstructionsAdmin(req, existing); + const entryFile = typeof req.query.path === "string" ? req.query.path : deriveBundleState(existing).entryFile; + const cursor = typeof req.query.cursor === "string" ? req.query.cursor : undefined; + if (cursor && !isUuidLike(cursor)) throw unprocessable("Invalid history cursor"); + res.json(await instructionRevisions.history({ companyId: existing.companyId, agentId: existing.id, entryFile, cursor }, req.actor)); + }); + router.get("/agents/:id/instructions-bundle/revision/:revisionId", async (req, res) => { + const existing = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Agent not found"); + if (!existing) return; + assertExternalInstructionsAdmin(req, existing); + const revisionId = req.params.revisionId as string; + if (!isUuidLike(revisionId)) throw unprocessable("Invalid instruction revision id"); + const entryFile = typeof req.query.path === "string" ? req.query.path : deriveBundleState(existing).entryFile; + res.json(await instructionRevisions.readRevision({ companyId: existing.companyId, agentId: existing.id, entryFile, revisionId }, req.actor)); + }); + router.get("/agents/:id/instructions-bundle/diff", async (req, res) => { + const existing = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Agent not found"); + if (!existing) return; + assertExternalInstructionsAdmin(req, existing); + const fromRevisionId = typeof req.query.from === "string" ? req.query.from : ""; + const toRevisionId = typeof req.query.to === "string" ? req.query.to : ""; + if (!isUuidLike(fromRevisionId) || !isUuidLike(toRevisionId)) throw unprocessable("Provide valid from and to revision ids"); + const entryFile = typeof req.query.path === "string" ? req.query.path : deriveBundleState(existing).entryFile; + res.json(await instructionRevisions.diff({ companyId: existing.companyId, agentId: existing.id, entryFile, fromRevisionId, toRevisionId }, req.actor)); + }); + router.post("/agents/:id/instructions-bundle/restore", validate(restoreAgentInstructionSchema), async (req, res) => { + const existing = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Agent not found"); + if (!existing) return; + assertExternalInstructionsAdmin(req, existing); + const receipt = await instructionRevisions.restore({ companyId: existing.companyId, agentId: existing.id, + entryFile: req.body.path, baseRevisionId: req.body.baseRevisionId, revisionId: req.body.revisionId }, req.actor); + res.json(instructionFileDetail(receipt, receipt)); + }); + router.delete("/agents/:id/instructions-bundle/file", async (req, res) => { const id = req.params.id as string; const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); diff --git a/server/src/routes/built-in-agents.ts b/server/src/routes/built-in-agents.ts index 9c16ba6f53b..c881ae9c93f 100644 --- a/server/src/routes/built-in-agents.ts +++ b/server/src/routes/built-in-agents.ts @@ -217,7 +217,7 @@ export function builtInAgentRoutes(db: Db) { const key = req.params.key as string; await assertBuiltInAgentsEnabled(); await assertCanProvisionBuiltInAgents(req, companyId); - const state = await svc.reset(companyId, key, req.body); + const state = await svc.reset(companyId, key, req.body, req.actor); await logBuiltInAgentMutation(req, { companyId, action: "built_in_agent.reset", diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 439262a6fce..df4dfbe4632 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -27,6 +27,7 @@ import { updateAgentInstructionsPathSchema, updateAgentInstructionsBundleSchema, upsertAgentInstructionsFileSchema, + restoreAgentInstructionSchema, createAgentKeySchema, builtInAgentEmptyMutationSchema, builtInAgentProvisionSchema, @@ -3471,6 +3472,19 @@ registry.registerPath({ responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized }, }); +for (const operation of [ + { suffix: "history", summary: "List immutable instruction revisions", query: z.object({ path: z.string().optional(), cursor: z.string().uuid().optional() }) }, + { suffix: "revision/{revisionId}", summary: "Read exact instruction content at a revision", query: z.object({ path: z.string().optional() }) }, + { suffix: "diff", summary: "Compare instruction revisions (exact common prefix, removed, added, suffix)", query: z.object({ path: z.string().optional(), from: z.string().uuid(), to: z.string().uuid() }) }, +]) { + registry.registerPath({ method: "get", path: `/api/agents/{id}/instructions-bundle/${operation.suffix}`, tags: ["agents"], summary: operation.summary, + request: { params: operation.suffix.includes("revisionId") ? z.object({ id: z.string(), revisionId: z.string().uuid() }) : z.object({ id: z.string() }), query: operation.query }, + responses: { 200: r.ok(), 401: r.unauthorized, 403: r.forbidden, 404: r.notFound } }); +} +registry.registerPath({ method: "post", path: "/api/agents/{id}/instructions-bundle/restore", tags: ["agents"], summary: "Restore instruction content as a new revision with compare-and-swap", + request: { params: z.object({ id: z.string() }), body: jsonBody(restoreAgentInstructionSchema) }, + responses: { 200: r.ok(), 401: r.unauthorized, 403: r.forbidden, 404: r.notFound, 409: r.conflict } }); + registry.registerPath({ method: "delete", path: "/api/agents/{id}/instructions-bundle/file", diff --git a/server/src/services/agent-instruction-authorization.ts b/server/src/services/agent-instruction-authorization.ts new file mode 100644 index 00000000000..21798941f3f --- /dev/null +++ b/server/src/services/agent-instruction-authorization.ts @@ -0,0 +1,175 @@ +import { and, eq, isNull } from "drizzle-orm"; +import { + agentApiKeys, + authUsers, + companyMemberships, + heartbeatRuns, + runIdentityContexts, + type Db, +} from "@paperclipai/db"; +import { forbidden } from "../errors.js"; +import { + authorizationService, + authorizationDeniedDetails, + type AuthorizationActor, +} from "./authorization.js"; +import { + agentInstructionsChangeTargetKey, + changeConsentGateService, +} from "./change-consent-gate.js"; + +type Connection = Db | Parameters[0]>[0]; + +/** Accept ONLY a server-authenticated actor, never request/tool JSON. Rebind run/key provenance on every call. */ +export async function resolveInstructionActor( + db: Connection, + actor: AuthorizationActor, +): Promise { + if (actor.type === "board") { + if (!actor.userId) + throw forbidden("Instruction edits require an authenticated user"); + // Discard caller membership caches so revoked access is observed. + return { + type: "board", + userId: actor.userId, + source: actor.source, + ignoreInstanceAdmin: actor.ignoreInstanceAdmin, + }; + } + if (actor.type !== "agent" || !actor.agentId || !actor.companyId) + throw forbidden("Instruction identity is missing"); + let responsibleUserId: string | null = null; + let keyScope = actor.keyScope; + if (actor.keyId) { + const [key] = await db + .select() + .from(agentApiKeys) + .where( + and( + eq(agentApiKeys.id, actor.keyId), + eq(agentApiKeys.companyId, actor.companyId), + eq(agentApiKeys.agentId, actor.agentId), + isNull(agentApiKeys.revokedAt), + ), + ); + if (!key) + throw forbidden("Instruction API key is missing or revoked", { + code: "INSTRUCTION_IDENTITY_INVALID", + }); + responsibleUserId = key.responsibleUserId; + keyScope = key.scopeConfig ?? undefined; + } + if (actor.runId) { + const [run] = await db + .select() + .from(heartbeatRuns) + .where( + and( + eq(heartbeatRuns.id, actor.runId), + eq(heartbeatRuns.companyId, actor.companyId), + eq(heartbeatRuns.agentId, actor.agentId), + ), + ); + if (!run) + throw forbidden("Instruction run identity does not match", { + code: "INSTRUCTION_IDENTITY_INVALID", + }); + if (actor.keyId && responsibleUserId !== run.responsibleUserId) + throw forbidden("Key and run responsible identity do not match", { + code: "INSTRUCTION_IDENTITY_INVALID", + }); + responsibleUserId = run.responsibleUserId; + if (run.activeIdentityContextId) { + const [context] = await db + .select() + .from(runIdentityContexts) + .where( + and( + eq(runIdentityContexts.id, run.activeIdentityContextId), + eq(runIdentityContexts.runId, run.id), + eq(runIdentityContexts.companyId, run.companyId), + eq(runIdentityContexts.status, "accepted"), + ), + ); + if (!context) + throw forbidden("Instruction run identity is unavailable", { + code: "INSTRUCTION_IDENTITY_INVALID", + }); + responsibleUserId = context.responsibleUserId; + } + } + if ( + !responsibleUserId || + (actor.onBehalfOfUserId && actor.onBehalfOfUserId !== responsibleUserId) + ) { + throw forbidden("A current server-bound responsible user is required", { + code: "INSTRUCTION_IDENTITY_INVALID", + }); + } + const [membership] = await db + .select({ userId: authUsers.id }) + .from(companyMemberships) + .innerJoin(authUsers, eq(authUsers.id, companyMemberships.principalId)) + .where( + and( + eq(companyMemberships.companyId, actor.companyId), + eq(companyMemberships.principalType, "user"), + eq(companyMemberships.principalId, responsibleUserId), + eq(companyMemberships.status, "active"), + ), + ); + if (!membership) + throw forbidden("The responsible user is unavailable in this company", { + code: "RESPONSIBLE_USER_UNAVAILABLE", + }); + return { + type: "agent", + agentId: actor.agentId, + companyId: actor.companyId, + runId: actor.runId, + keyId: actor.keyId, + keyScope, + source: actor.source, + onBehalfOfUserId: responsibleUserId, + }; +} + +/** Current target edit access + agent containment + protected-change consent, shared by all writers. */ +export async function authorizeInstructionCommit( + db: Connection, + actor: AuthorizationActor, + target: { companyId: string; id: string }, +) { + const bound = await resolveInstructionActor(db, actor); + const access = authorizationService(db); + const input = { + actor: bound, + action: "agent_instructions:update" as const, + resource: { + type: "agent" as const, + companyId: target.companyId, + agentId: target.id, + }, + scope: { targetAgentId: target.id, requiresChangeGrant: true }, + }; + let decision = await access.decide(input); + if ( + !decision.allowed && + decision.reason === "deny_missing_consent" && + bound.type === "agent" + ) { + await changeConsentGateService(db).assertConsented({ + companyId: target.companyId, + actorAgentId: bound.agentId, + actorRunId: bound.runId, + targetKeys: [agentInstructionsChangeTargetKey(target.id)], + }); + decision = await access.decide({ + ...input, + scope: { ...input.scope, consentedChange: true }, + }); + } + if (!decision.allowed) + throw forbidden(decision.explanation, authorizationDeniedDetails(decision)); + return bound; +} diff --git a/server/src/services/agent-instruction-files.ts b/server/src/services/agent-instruction-files.ts new file mode 100644 index 00000000000..c839ec3ecab --- /dev/null +++ b/server/src/services/agent-instruction-files.ts @@ -0,0 +1,149 @@ +import fs from "node:fs/promises"; +import { constants } from "node:fs"; +import path from "node:path"; +import { randomUUID } from "node:crypto"; +import { unprocessable } from "../errors.js"; + +export const MAX_INSTRUCTION_BYTES = 1024 * 1024; +export function instructionPath(value: string): string { + if ( + !value || + value.length > 512 || + value.includes("\0") || + value.includes("\\") || + path.posix.isAbsolute(value) || + /^[a-z]:/i.test(value) || + value.split("/").some((part) => !part || part === "." || part === "..") + ) { + throw unprocessable("Use a relative instructions path without traversal", { + code: "INSTRUCTION_PATH_INVALID", + }); + } + return value; +} + +export function instructionBytes(content: string | Uint8Array): Buffer { + const byteLength = + typeof content === "string" + ? Buffer.byteLength(content, "utf8") + : content.byteLength; + if (byteLength > MAX_INSTRUCTION_BYTES) { + throw unprocessable("Instructions exceed the 1 MiB limit", { + code: "INSTRUCTION_CONTENT_INVALID", + }); + } + const bytes = + typeof content === "string" + ? Buffer.from(content, "utf8") + : Buffer.from(content); + try { + // Preserve BOM; reject invalid UTF-8 and unpaired JS surrogates instead of replacing bytes. + const decoded = new TextDecoder("utf-8", { + fatal: true, + ignoreBOM: true, + }).decode(bytes); + if (typeof content === "string" && decoded !== content) + throw new Error("invalid Unicode"); + } catch { + throw unprocessable("Instructions must be valid UTF-8", { + code: "INSTRUCTION_CONTENT_INVALID", + }); + } + return bytes; +} + +export async function assertInstructionPathSafe( + root: string, + entryFile: string, +) { + instructionPath(entryFile); + const absolute = path.resolve(root, entryFile); + let current = path.parse(absolute).root; + const parts = absolute.slice(current.length).split(path.sep); + for (const [i, part] of parts.entries()) { + current = path.join(current, part); + const stat = await fs + .lstat(current) + .catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return null; + throw error; + }); + if (!stat) continue; + if ( + stat.isSymbolicLink() || + (i < parts.length - 1 ? !stat.isDirectory() : !stat.isFile()) + ) { + throw unprocessable( + "Instructions require regular files and directories without symlinks", + { code: "INSTRUCTION_PATH_INVALID" }, + ); + } + } + return absolute; +} + +export async function readInstructionBytes( + root: string, + entryFile: string, +): Promise { + const absolute = await assertInstructionPathSafe(root, entryFile); + const file = await fs + .open(absolute, constants.O_RDONLY | constants.O_NOFOLLOW) + .catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return null; + throw error; + }); + if (!file) return null; + try { + const stat = await file.stat(); + if (!stat.isFile() || stat.size > MAX_INSTRUCTION_BYTES) { + throw unprocessable( + "Instructions must be a regular UTF-8 file of at most 1 MiB", + { code: "INSTRUCTION_CONTENT_INVALID" }, + ); + } + const buffer = Buffer.alloc(MAX_INSTRUCTION_BYTES + 1); + let length = 0; + while (length < buffer.length) { + const result = await file.read( + buffer, + length, + buffer.length - length, + length, + ); + if (result.bytesRead === 0) break; + length += result.bytesRead; + } + return instructionBytes(buffer.subarray(0, length)); + } finally { + await file.close(); + } +} + +/** Call while holding the canonical agent lock. Never writes through a symlink. */ +export async function materializeInstructionBytes( + root: string, + entryFile: string, + bytes: Buffer, +) { + const absolute = await assertInstructionPathSafe(root, entryFile); + await fs.mkdir(path.dirname(absolute), { recursive: true }); + await assertInstructionPathSafe(root, entryFile); + const temporary = path.join( + path.dirname(absolute), + `.instruction-${randomUUID()}.tmp`, + ); + try { + const file = await fs.open(temporary, "wx", 0o600); + try { + await file.writeFile(bytes); + await file.sync(); + } finally { + await file.close(); + } + await assertInstructionPathSafe(root, entryFile); + await fs.rename(temporary, absolute); + } finally { + await fs.rm(temporary, { force: true }); + } +} diff --git a/server/src/services/agent-instruction-revisions.ts b/server/src/services/agent-instruction-revisions.ts new file mode 100644 index 00000000000..a521e378e92 --- /dev/null +++ b/server/src/services/agent-instruction-revisions.ts @@ -0,0 +1,469 @@ +import { createHash } from "node:crypto"; +import { and, desc, eq, getTableColumns, lt, or, sql } from "drizzle-orm"; +import { + activityLog, + agents, + agentInstructionHeads as heads, + agentInstructionRevisions as revisions, + type Db, +} from "@paperclipai/db"; +import type { + AgentInstructionCommitReceipt, + AgentInstructionDiff, + AgentInstructionHistory, + AgentInstructionSnapshot, + AgentInstructionSource, +} from "@paperclipai/shared"; +import { conflict, forbidden, notFound, unprocessable } from "../errors.js"; +import { + agentInstructionsBundleMode, + deriveBundleState, + resolveManagedInstructionsRoot, +} from "./agent-instructions.js"; +import { + instructionBytes, + instructionPath, + assertInstructionPathSafe, + readInstructionBytes, + materializeInstructionBytes, +} from "./agent-instruction-files.js"; +import { + authorizeInstructionCommit, + resolveInstructionActor, +} from "./agent-instruction-authorization.js"; +import { + authorizationService, + authorizationDeniedDetails, + type AuthorizationActor, +} from "./authorization.js"; + +type Tx = Parameters[0]>[0]; +type Revision = typeof revisions.$inferSelect; +const { contentBase64: _contentColumn, ...revisionMetadataColumns } = + getTableColumns(revisions); +export type InstructionTarget = { companyId: string; agentId: string }; +export type InstructionCommitInput = InstructionTarget & { + entryFile: string; + content: string | Uint8Array; + baseRevisionId: string | null; + source: Exclude; +}; +function metadata(row: Omit) { + const { createdAt, source, ...fields } = row; + return { + ...fields, + source: source as AgentInstructionSource, + createdAt: createdAt.toISOString(), + }; +} +function snapshot(row: Revision): AgentInstructionSnapshot { + const { contentBase64, ...fields } = row; + return { + revision: metadata(fields), + content: Buffer.from(contentBase64, "base64").toString("utf8"), + }; +} +function owner(target: InstructionTarget, entryFile: string) { + return and( + eq(revisions.companyId, target.companyId), + eq(revisions.agentId, target.agentId), + eq(revisions.entryFile, entryFile), + ); +} +function headOwner(target: InstructionTarget, entryFile: string) { + return and( + eq(heads.companyId, target.companyId), + eq(heads.agentId, target.agentId), + eq(heads.entryFile, entryFile), + ); +} + +export function agentInstructionRevisionService(db: Db) { + async function lockTarget( + tx: Tx, + target: InstructionTarget, + actor?: AuthorizationActor, + ) { + if (actor?.type === "agent" && actor.companyId !== target.companyId) + throw notFound("Agent not found"); + const [agent] = await tx + .select() + .from(agents) + .where( + and( + eq(agents.id, target.agentId), + eq(agents.companyId, target.companyId), + ), + ) + .for("update"); + if (!agent) throw notFound("Agent not found"); + if (agentInstructionsBundleMode(agent) === "external") { + throw unprocessable( + "External host instructions do not support revision write-back. Ask an instance administrator to migrate this bundle to managed storage.", + { code: "INSTRUCTION_MANAGED_BUNDLE_REQUIRED" }, + ); + } + const state = deriveBundleState(agent); + const entryFile = instructionPath(state.entryFile); + return { agent, entryFile, root: resolveManagedInstructionsRoot(agent) }; + } + async function authorizeRead( + tx: Tx, + actor: AuthorizationActor, + target: InstructionTarget, + ) { + const bound = await resolveInstructionActor(tx, actor); + const decision = await authorizationService(tx).decide({ + actor: bound, + action: "agent:read", + resource: { + type: "agent", + companyId: target.companyId, + agentId: target.agentId, + }, + }); + if (!decision.allowed) + throw forbidden( + decision.explanation, + authorizationDeniedDetails(decision), + ); + return bound; + } + async function head(tx: Tx, target: InstructionTarget, entryFile: string) { + const [result] = await tx + .select({ revision: revisions }) + .from(heads) + .innerJoin(revisions, eq(revisions.id, heads.revisionId)) + .where(headOwner(target, entryFile)); + return result?.revision ?? null; + } + async function append( + tx: Tx, + target: InstructionTarget, + entryFile: string, + bytes: Buffer, + actor: AuthorizationActor, + source: AgentInstructionSource, + parentRevisionId: string | null, + baseRevisionId: string | null, + restoredFromRevisionId: string | null = null, + ) { + const [row] = await tx + .insert(revisions) + .values({ + ...target, + entryFile, + contentBase64: bytes.toString("base64"), + contentHash: createHash("sha256").update(bytes).digest("hex"), + byteLength: bytes.length, + parentRevisionId, + baseRevisionId, + restoredFromRevisionId, + source, + actorAgentId: actor.type === "agent" ? actor.agentId : null, + actorUserId: actor.type === "board" ? actor.userId : null, + responsibleUserId: + actor.type === "board" ? actor.userId : actor.onBehalfOfUserId, + sourceRunId: actor.runId, + }) + .returning(); + await tx + .insert(heads) + .values({ ...target, entryFile, revisionId: row.id }) + .onConflictDoUpdate({ + target: [heads.companyId, heads.agentId, heads.entryFile], + set: { revisionId: row.id, updatedAt: new Date() }, + }); + await tx.insert(activityLog).values({ + companyId: target.companyId, + actorType: actor.type === "board" ? "user" : "agent", + actorId: (actor.type === "board" ? actor.userId : actor.agentId)!, + agentId: actor.type === "agent" ? actor.agentId : null, + runId: actor.runId, + responsibleUserId: row.responsibleUserId, + action: "agent.instructions_revision_committed", + entityType: "agent", + entityId: target.agentId, + details: { + revisionId: row.id, + entryFile, + source, + parentRevisionId, + baseRevisionId, + contentHash: row.contentHash, + byteLength: row.byteLength, + }, + }); + return row; + } + async function seed( + tx: Tx, + target: InstructionTarget, + state: Awaited>, + actor: AuthorizationActor, + ) { + const current = await head(tx, target, state.entryFile); + if (current) return current; + const bytes = await readInstructionBytes(state.root, state.entryFile); + // Never scan for a fallback entry. An unconfigured legacy inline prompt is + // migrated only on an explicit content commit; reads preserve configured bytes. + if (!bytes) return null; + return append( + tx, + target, + state.entryFile, + bytes, + actor, + "seed", + null, + null, + ); + } + async function getRevision( + tx: Tx, + target: InstructionTarget, + entryFile: string, + id: string, + ) { + const [row] = await tx + .select() + .from(revisions) + .where(and(owner(target, entryFile), eq(revisions.id, id))); + if (!row) throw notFound("Instruction revision not found"); + return row; + } + async function readCurrent( + target: InstructionTarget, + actor: AuthorizationActor, + ): Promise { + return db.transaction(async (tx) => { + const state = await lockTarget(tx, target, actor); + const bound = await authorizeRead(tx, actor, target); + const row = await seed(tx, target, state, bound); + return row ? snapshot(row) : null; + }); + } + /** Rebuild disk from the current committed head under the same lock as commits. Safe after restart. */ + async function materializeCurrent(target: InstructionTarget): Promise { + await db.transaction(async (tx) => { + const state = await lockTarget(tx, target); + const current = await head(tx, target, state.entryFile); + if (!current) throw notFound("Instruction head not found"); + await materializeInstructionBytes( + state.root, + state.entryFile, + Buffer.from(current.contentBase64, "base64"), + ); + }); + } + async function commitInternal( + input: + | InstructionCommitInput + | (Omit & { + restoreRevisionId: string; + }), + actor: AuthorizationActor, + ): Promise { + instructionPath(input.entryFile); + const bytes = "content" in input ? instructionBytes(input.content) : null; + const result = await db.transaction(async (tx) => { + const state = await lockTarget(tx, input, actor); + const bound = await authorizeInstructionCommit(tx, actor, state.agent); + if (state.entryFile !== input.entryFile) + throw conflict( + "Configured instruction entry changed; read the current entry and retry", + { code: "INSTRUCTION_ENTRY_CHANGED", entryFile: state.entryFile }, + ); + await assertInstructionPathSafe(state.root, state.entryFile); + const persistedHead = await head(tx, input, state.entryFile); + const current = persistedHead ?? (await seed(tx, input, state, bound)); + const restored = + "restoreRevisionId" in input + ? await getRevision( + tx, + input, + input.entryFile, + input.restoreRevisionId, + ) + : null; + const candidate = bytes ?? Buffer.from(restored!.contentBase64, "base64"); + // An exact replay can safely return the durable receipt even after its base + // advanced. Restore also deduplicates identical content, preserving history. + if (current && current.contentBase64 === candidate.toString("base64")) + return { row: current, changed: false }; + if ((current?.id ?? null) !== input.baseRevisionId) { + throw conflict( + "Instructions changed since the base revision. Read the current entry before saving.", + { + code: "INSTRUCTION_REVISION_CONFLICT", + baseRevisionId: input.baseRevisionId, + // A seed created in this transaction rolls back with the conflict. + currentRevisionId: persistedHead?.id ?? null, + }, + ); + } + const row = await append( + tx, + input, + input.entryFile, + candidate, + bound, + restored ? "restore" : (input as InstructionCommitInput).source, + current?.id ?? null, + input.baseRevisionId, + restored?.id ?? null, + ); + if ( + !state.agent.adapterConfig.instructionsRootPath || + !state.agent.adapterConfig.instructionsBundleMode + ) { + await tx + .update(agents) + .set({ + adapterConfig: { + ...state.agent.adapterConfig, + instructionsBundleMode: "managed", + instructionsRootPath: state.root, + instructionsEntryFile: state.entryFile, + instructionsFilePath: `${state.root}/${state.entryFile}`, + }, + updatedAt: new Date(), + }) + .where(eq(agents.id, state.agent.id)); + } + return { row, changed: true }; + }); + let materialization: AgentInstructionCommitReceipt["materialization"] = + "current"; + try { + await materializeCurrent(input); + } catch { + materialization = "pending"; + } + return { + ...snapshot(result.row), + changed: result.changed, + materialization, + }; + } + async function history( + target: InstructionTarget & { + entryFile: string; + cursor?: string; + limit?: number; + }, + actor: AuthorizationActor, + ): Promise { + const limit = Math.max(1, Math.min(100, target.limit ?? 50)); + return db.transaction(async (tx) => { + const state = await lockTarget(tx, target, actor); + const bound = await authorizeRead(tx, actor, target); + await seed(tx, target, state, bound); + instructionPath(target.entryFile); + const cursor = target.cursor + ? await getRevision(tx, target, target.entryFile, target.cursor) + : null; + // Keep PostgreSQL timestamp precision; JS Date truncates microseconds and + // would skip revisions in the cursor's fractional millisecond. + const cursorTime = cursor + ? sql`(select created_at from agent_instruction_revisions where id = ${cursor.id})` + : null; + const rows = await tx + .select(revisionMetadataColumns) + .from(revisions) + .where( + and( + owner(target, target.entryFile), + cursor && cursorTime + ? or( + lt(revisions.createdAt, cursorTime), + and( + eq(revisions.createdAt, cursorTime), + lt(revisions.id, cursor.id), + ), + ) + : undefined, + ), + ) + .orderBy(desc(revisions.createdAt), desc(revisions.id)) + .limit(limit + 1); + return { + revisions: rows.slice(0, limit).map(metadata), + nextCursor: rows.length > limit ? rows[limit - 1]!.id : null, + }; + }); + } + async function readRevision( + target: InstructionTarget & { entryFile: string; revisionId: string }, + actor: AuthorizationActor, + ) { + return db.transaction(async (tx) => { + await lockTarget(tx, target, actor); + await authorizeRead(tx, actor, target); + return snapshot( + await getRevision( + tx, + target, + instructionPath(target.entryFile), + target.revisionId, + ), + ); + }); + } + async function diff( + target: InstructionTarget & { + entryFile: string; + fromRevisionId: string; + toRevisionId: string; + }, + actor: AuthorizationActor, + ): Promise { + const from = await readRevision( + { ...target, revisionId: target.fromRevisionId }, + actor, + ); + const to = await readRevision( + { ...target, revisionId: target.toRevisionId }, + actor, + ); + // Linear, bounded exact replacement diff. Code points keep surrogate pairs intact. + const a = Array.from(from.content), + b = Array.from(to.content); + let start = 0, + end = 0; + while (start < a.length && start < b.length && a[start] === b[start]) + start++; + while ( + end < a.length - start && + end < b.length - start && + a[a.length - end - 1] === b[b.length - end - 1] + ) + end++; + return { + from, + to, + prefix: a.slice(0, start).join(""), + removed: a.slice(start, a.length - end).join(""), + added: b.slice(start, b.length - end).join(""), + suffix: end ? a.slice(-end).join("") : "", + }; + } + return { + readCurrent, + readRevision, + history, + diff, + materializeCurrent, + commit: (input: InstructionCommitInput, actor: AuthorizationActor) => + commitInternal(input, actor), + restore: ( + input: InstructionTarget & { + entryFile: string; + baseRevisionId: string; + revisionId: string; + }, + actor: AuthorizationActor, + ) => + commitInternal({ ...input, restoreRevisionId: input.revisionId }, actor), + }; +} diff --git a/server/src/services/agent-instructions.ts b/server/src/services/agent-instructions.ts index 5045143eb70..43c0a6e63ec 100644 --- a/server/src/services/agent-instructions.ts +++ b/server/src/services/agent-instructions.ts @@ -1,5 +1,8 @@ import fs from "node:fs/promises"; import path from "node:path"; +import { and, eq } from "drizzle-orm"; +import { agentInstructionHeads, agents, type Db } from "@paperclipai/db"; +import { instructionPath, assertInstructionPathSafe, instructionBytes, readInstructionBytes } from "./agent-instruction-files.js"; import { notFound, unprocessable } from "../errors.js"; import { resolveHomeAwarePath, resolvePaperclipInstanceRoot } from "../home-paths.js"; @@ -112,11 +115,7 @@ function isMarkdown(relativePath: string) { } function normalizeRelativeFilePath(candidatePath: string): string { - const normalized = path.posix.normalize(candidatePath.replaceAll("\\", "/")).replace(/^\/+/, ""); - if (!normalized || normalized === "." || normalized === ".." || normalized.startsWith("../")) { - throw unprocessable("Instructions file path must stay within the bundle root"); - } - return normalized; + return instructionPath(candidatePath); } function resolvePathWithinRoot(rootPath: string, relativePath: string): string { @@ -130,7 +129,7 @@ function resolvePathWithinRoot(rootPath: string, relativePath: string): string { return absolutePath; } -function resolveManagedInstructionsRoot(agent: AgentLike): string { +export function resolveManagedInstructionsRoot(agent: AgentLike): string { return path.resolve( resolvePaperclipInstanceRoot(), "companies", @@ -153,7 +152,7 @@ function resolveLegacyInstructionsPath(candidatePath: string, config: Record null); + return fs.lstat(targetPath).catch(() => null); } function shouldIgnoreInstructionsEntry(entry: { name: string; isDirectory(): boolean; isFile(): boolean }) { @@ -231,7 +230,7 @@ async function readLegacyInstructions(agent: AgentLike, config: Record[0]>[0] | undefined = db) { + if (!connection) throw unprocessable("Bundle initialization requires the database-backed instructions service"); + const [head] = await connection.select().from(agentInstructionHeads).where(and(eq(agentInstructionHeads.companyId, agent.companyId), + eq(agentInstructionHeads.agentId, agent.id), eq(agentInstructionHeads.entryFile, entryFile))); + if (head) throw unprocessable("This entry has revision history. Use the instruction content API with its baseRevisionId to update it.", { code: "INSTRUCTION_REVISION_REQUIRED" }); + } async function getBundle(agent: AgentLike): Promise { const state = await recoverManagedBundleState(agent, deriveBundleState(agent)); if (!state.rootPath) return toBundle(agent, state, []); + await assertInstructionPathSafe(state.rootPath, state.entryFile); const stat = await statIfExists(state.rootPath); if (!stat?.isDirectory()) { return toBundle(agent, { @@ -507,6 +510,7 @@ export function agentInstructionsService() { }; } if (!state.rootPath) throw notFound("Agent instructions bundle is not configured"); + await assertInstructionPathSafe(state.rootPath, relativePath); const absolutePath = resolvePathWithinRoot(state.rootPath, relativePath); const [content, stat] = await Promise.all([ fs.readFile(absolutePath, "utf8").catch(() => null), @@ -551,11 +555,12 @@ export function agentInstructionsService() { }); await fs.mkdir(managedRoot, { recursive: true }); - const entryPath = resolvePathWithinRoot(managedRoot, entryFile); + const entryPath = await assertInstructionPathSafe(managedRoot, entryFile); const entryStat = await statIfExists(entryPath); if (!entryStat?.isFile()) { const legacyInstructions = await readLegacyInstructions(agent, current.config); if (legacyInstructions.trim().length > 0) { + await assertUnversionedEntry(agent, entryFile); await fs.mkdir(path.dirname(entryPath), { recursive: true }); await fs.writeFile(entryPath, legacyInstructions, "utf8"); } @@ -595,9 +600,13 @@ export function agentInstructionsService() { nextRootPath = resolvedRoot; } + await assertInstructionPathSafe(nextRootPath, nextEntryFile); await fs.mkdir(nextRootPath, { recursive: true }); const existingFiles = await listFilesRecursive(nextRootPath); + if (nextMode === "managed" && !existingFiles.includes(nextEntryFile)) { + await assertUnversionedEntry(agent, nextEntryFile); + } const exported = await exportFiles(agent); if (existingFiles.length === 0) { await writeBundleFiles(nextRootPath, exported.files); @@ -618,7 +627,7 @@ export function agentInstructionsService() { return { bundle: nextBundle, adapterConfig: nextConfig }; } - async function writeFile( + async function writeFileUnversioned( agent: AgentLike, relativePath: string, content: string, @@ -642,7 +651,13 @@ export function agentInstructionsService() { return { bundle, file, adapterConfig }; } + const configured = await recoverManagedBundleState(agent, current); + if (normalizeRelativeFilePath(relativePath) === configured.entryFile) { + throw unprocessable("Entry edits require the canonical instruction commit service and baseRevisionId", { code: "INSTRUCTION_REVISION_REQUIRED" }); + } const prepared = await ensureWritableBundle(agent, options); + instructionBytes(content); + await assertInstructionPathSafe(prepared.state.rootPath!, relativePath); const absolutePath = resolvePathWithinRoot(prepared.state.rootPath!, relativePath); await fs.mkdir(path.dirname(absolutePath), { recursive: true }); await fs.writeFile(absolutePath, content, "utf8"); @@ -654,7 +669,7 @@ export function agentInstructionsService() { return { bundle, file, adapterConfig: prepared.adapterConfig }; } - async function deleteFile(agent: AgentLike, relativePath: string): Promise<{ + async function deleteFileUnversioned(agent: AgentLike, relativePath: string): Promise<{ bundle: AgentInstructionsBundle; adapterConfig: Record; }> { @@ -668,13 +683,35 @@ export function agentInstructionsService() { if (normalizedPath === state.entryFile) { throw unprocessable("Cannot delete the bundle entry file"); } - const absolutePath = resolvePathWithinRoot(state.rootPath, normalizedPath); + const absolutePath = await assertInstructionPathSafe(state.rootPath, normalizedPath); await fs.rm(absolutePath, { force: true }); const adapterConfig = buildPersistedBundleConfig(derived, state); const bundle = await getBundle({ ...agent, adapterConfig }); return { bundle, adapterConfig }; } + // Reload configuration under the commit lock: a previously supporting file + // may have become the configured entry since the route loaded its agent. + async function withCurrentAgent(agent: AgentLike, relativePath: string, operation: (current: AgentLike) => Promise) { + if (!db) return operation(agent); + return db.transaction(async (tx) => { + const [current] = await tx.select().from(agents) + .where(and(eq(agents.id, agent.id), eq(agents.companyId, agent.companyId))).for("update"); + if (!current) throw notFound("Agent not found"); + if (relativePath !== LEGACY_PROMPT_TEMPLATE_PATH) { + // Historical entries also remain projections if the configured entry changes. + await assertUnversionedEntry(current, normalizeRelativeFilePath(relativePath), tx); + } + return operation(current); + }); + } + async function writeFile(...args: Parameters) { + return withCurrentAgent(args[0], args[1], (current) => writeFileUnversioned(current, args[1], args[2], args[3])); + } + async function deleteFile(...args: Parameters) { + return withCurrentAgent(args[0], args[1], (current) => deleteFileUnversioned(current, args[1])); + } + async function exportFiles(agent: AgentLike, options?: { rejectSymlinks?: boolean }): Promise<{ files: Record; entryFile: string; @@ -704,7 +741,7 @@ export function agentInstructionsService() { }; } - async function materializeManagedBundle( + async function materializeManagedBundleUnversioned( agent: AgentLike, files: Record, options?: { @@ -716,6 +753,14 @@ export function agentInstructionsService() { const rootPath = resolveManagedInstructionsRoot(agent); const entryFile = options?.entryFile ? normalizeRelativeFilePath(options.entryFile) : ENTRY_FILE_DEFAULT; + for (const [relativePath, content] of Object.entries(files)) { + instructionBytes(content); + await assertInstructionPathSafe(rootPath, relativePath); + } + const previous = await readInstructionBytes(rootPath, entryFile); + if (previous && !previous.equals(instructionBytes(files[entryFile] ?? ""))) { + throw unprocessable("Existing entry content must be saved through the canonical revision API before replacing a bundle", { code: "INSTRUCTION_REVISION_REQUIRED" }); + } if (options?.replaceExisting) { await fs.rm(rootPath, { recursive: true, force: true }); } @@ -744,6 +789,24 @@ export function agentInstructionsService() { return { bundle, adapterConfig }; } + async function materializeManagedBundle( + ...args: Parameters + ) { + if (!db) throw unprocessable("Bundle initialization requires the database-backed instructions service"); + return db.transaction(async (tx) => { + const agent = args[0]; + const [owner] = await tx.select({ id: agents.id }).from(agents).where(and(eq(agents.id, agent.id), eq(agents.companyId, agent.companyId))).for("update"); + if (!owner) throw notFound("Agent not found"); + await assertUnversionedEntry(agent, args[2]?.entryFile ? normalizeRelativeFilePath(args[2].entryFile) : ENTRY_FILE_DEFAULT, tx); + if (args[2]?.replaceExisting) { + const [existingHead] = await tx.select({ id: agentInstructionHeads.revisionId }).from(agentInstructionHeads) + .where(and(eq(agentInstructionHeads.companyId, agent.companyId), eq(agentInstructionHeads.agentId, agent.id))).limit(1); + if (existingHead) throw unprocessable("Replacing this bundle would remove a versioned entry. Use canonical content commits.", { code: "INSTRUCTION_REVISION_REQUIRED" }); + } + return materializeManagedBundleUnversioned(...args); + }); + } + return { getBundle, readFile, diff --git a/server/src/services/authorization.ts b/server/src/services/authorization.ts index 70b1d2032cf..9c4305e2410 100644 --- a/server/src/services/authorization.ts +++ b/server/src/services/authorization.ts @@ -62,6 +62,7 @@ export type AuthorizationAction = | PermissionKey | "agent_config:read" | "agent_config:update" + | "agent_instructions:update" | "skill_config:update" | "agent:read" | "agent:wake" @@ -148,7 +149,7 @@ function companyIdForResource(resource: AuthorizationResource) { } function permissionForAction(action: AuthorizationAction): PermissionKey | null { - if (action === "agent_config:read" || action === "agent_config:update" || action === "skill_config:update") { + if (action === "agent_config:read" || action === "agent_config:update" || action === "agent_instructions:update" || action === "skill_config:update") { return null; } if ( @@ -998,6 +999,7 @@ export function authorizationService(db: Db | DbTransaction) { input.action === "decision_triage:manage" || input.action === "agent_config:read" || input.action === "agent_config:update" || + input.action === "agent_instructions:update" || input.action === "skill_config:update" || input.action === "inbox:manage" || input.action === "runtime:manage" || @@ -1734,7 +1736,7 @@ export function authorizationService(db: Db | DbTransaction) { if (input.action === "agent_config:read") { return decideWithAgentConfigReadGrant("user", input.actor.userId); } - if (input.action === "agent_config:update") { + if (input.action === "agent_config:update" || input.action === "agent_instructions:update") { return decideWithProtectedChangeGrants("user", input.actor.userId, { direct: "agents:configure", suggest: "agents:suggest-changes", @@ -2227,6 +2229,19 @@ export function authorizationService(db: Db | DbTransaction) { return decideWithAgentConfigReadGrant("agent", actorAgentId); } + if (input.action === "agent_instructions:update") { + if (!isSimpleAssignableAgentStatus(actorAgent.status) || !input.actor.onBehalfOfUserId) { + return deny({ action: input.action, reason: "deny_missing_membership", explanation: "Instruction edits require an active agent and a responsible user." }); + } + // Explicit configure/suggest restrictions still govern content changes. An + // ordinary standard agent with no such grants inherits its user's edit access. + const restricted = await decideWithProtectedChangeGrants("agent", actorAgentId, { + direct: "agents:configure", suggest: "agents:suggest-changes", + }); + if (restricted.reason !== "deny_no_grant") return restricted; + return allow({ action: input.action, reason: "allow_company_agent", explanation: "Standard agent content edit, subject to the responsible user's target edit access." }); + } + if (input.action === "agent_config:update") { return decideWithProtectedChangeGrants("agent", actorAgentId, { direct: "agents:configure", @@ -2391,7 +2406,7 @@ export function authorizationService(db: Db | DbTransaction) { responsibleUserId, }, "responsible-user authorization intersection denied"); - return responsibleUserAuthzShadowMode() ? agentDecision : denied; + return input.action !== "agent_instructions:update" && responsibleUserAuthzShadowMode() ? agentDecision : denied; } async function decide(input: { diff --git a/server/src/services/built-in-agents.ts b/server/src/services/built-in-agents.ts index 718263668eb..86e035e627d 100644 --- a/server/src/services/built-in-agents.ts +++ b/server/src/services/built-in-agents.ts @@ -10,7 +10,9 @@ import { syncRoutineVariablesWithTemplate } from "@paperclipai/shared"; import type { Agent, Approval, CompanySkill, PermissionKey, Routine, RoutineTrigger, RoutineVariable } from "@paperclipai/shared"; import { conflict, HttpError, notFound, unprocessable } from "../errors.js"; import { logActivity } from "./activity-log.js"; -import { agentInstructionsService } from "./agent-instructions.js"; +import { agentInstructionRevisionService } from "./agent-instruction-revisions.js"; +import type { AuthorizationActor } from "./authorization.js"; +import { agentInstructionsBundleMode, agentInstructionsService } from "./agent-instructions.js"; import { agentService } from "./agents.js"; import { approvalService } from "./approvals.js"; import { @@ -802,7 +804,7 @@ export function builtInAgentService(db: Db) { const agentSvc = agentService(db); const accessSvc = accessService(db); const approvalSvc = approvalService(db); - const instructionsSvc = agentInstructionsService(); + const instructionsSvc = agentInstructionsService(db); const skillSvc = companySkillService(db); const routineSvc = routineService(db); @@ -952,10 +954,15 @@ export function builtInAgentService(db: Db) { return currentFiles; } - async function materializeInstructions(agent: Agent, definition: BuiltInAgentDefinition, mode: "reconcile" | "reset") { + async function materializeInstructions(agent: Agent, definition: BuiltInAgentDefinition, mode: "reconcile" | "reset", actor?: AuthorizationActor) { const bundle = definition.bundle!; const stock = stockHash(bundle.instructions.files); const binding = await getManagedResourceBinding(agent.companyId, definition.key, "instructions", "AGENTS.md"); + // Stock operations also read the committed projection after interrupted writes. + if (agentInstructionsBundleMode(agent) === "managed") { + try { await agentInstructionRevisionService(db).materializeCurrent({ companyId: agent.companyId, agentId: agent.id }); } + catch (error) { if (!(error instanceof HttpError && error.status === 404)) throw error; } + } const currentFiles = await currentInstructionFiles(agent, bundle); const currentHash = Object.values(currentFiles).some((value) => value === null) ? null : stockHash(currentFiles); const currentState = stockState({ @@ -992,13 +999,41 @@ export function builtInAgentService(db: Db) { return currentState; } - const materialized = await instructionsSvc.materializeManagedBundle(agent, bundle.instructions.files, { - entryFile: bundle.instructions.entryFile, - replaceExisting: true, - clearLegacyPromptTemplate: true, - }); + let adapterConfig: Record; + if (currentFiles[bundle.instructions.entryFile] !== null && currentFiles[bundle.instructions.entryFile] !== undefined) { + // Automatic stock reconciliation cannot invent responsible-user authority. + // The stock update remains visible until an authenticated reset applies it. + if (!actor && mode === "reconcile") return currentState; + if (!actor) throw unprocessable("Resetting existing instructions requires an authenticated operator", { code: "INSTRUCTION_IDENTITY_INVALID" }); + const revisions = agentInstructionRevisionService(db); + const target = { companyId: agent.companyId, agentId: agent.id }; + const baseline = await revisions.readCurrent(target, actor); + const receipt = await revisions.commit({ ...target, entryFile: bundle.instructions.entryFile, + baseRevisionId: baseline?.revision.id ?? null, content: bundle.instructions.files[bundle.instructions.entryFile] ?? "", + source: actor.type === "board" ? "board" : "api" }, actor); + if (receipt.materialization === "pending") throw conflict("Instruction revision saved; retry reset to repair its disk copy", { revisionId: receipt.revision.id }); + const refreshed = await agentSvc.getById(agent.id); + if (!refreshed) throw notFound("Built-in agent not found"); + for (const [file, content] of Object.entries(bundle.instructions.files)) { + if (file !== bundle.instructions.entryFile) await instructionsSvc.writeFile(refreshed, file, content); + } + const currentBundle = await instructionsSvc.getBundle(refreshed); + for (const file of currentBundle.files) { + if (!file.isEntryFile && !file.virtual && !(file.path in bundle.instructions.files)) await instructionsSvc.deleteFile(refreshed, file.path); + } + adapterConfig = { ...refreshed.adapterConfig }; + delete adapterConfig.promptTemplate; + delete adapterConfig.bootstrapPromptTemplate; + } else { + const materialized = await instructionsSvc.materializeManagedBundle(agent, bundle.instructions.files, { + entryFile: bundle.instructions.entryFile, + replaceExisting: true, + clearLegacyPromptTemplate: true, + }); + adapterConfig = materialized.adapterConfig; + } const updated = await agentSvc.update(agent.id, { - adapterConfig: materialized.adapterConfig, + adapterConfig, }, { allowBuiltInAgentMetadata: true, recordRevision: { source: `built-in-bundle:${mode}:instructions` }, @@ -1480,13 +1515,14 @@ export function builtInAgentService(db: Db) { definition: BuiltInAgentDefinition, mode: "reconcile" | "reset", resources?: Array<"instructions" | "skill" | "routine">, + actor?: AuthorizationActor, ) { if (!definition.bundle) return []; const selected = new Set(resources ?? ["instructions", "skill", "routine"]); const existingStates = await bundleResourceStates(agent.companyId, definition, agent); const byKind = new Map(existingStates.map((state) => [state.resourceKind, state])); const instruction = selected.has("instructions") - ? await materializeInstructions(agent, definition, mode) + ? await materializeInstructions(agent, definition, mode, actor) : byKind.get("instructions")!; const refreshedAgent = await agentSvc.getById(agent.id) as Agent | null; if (!refreshedAgent) throw notFound("Built-in agent not found"); @@ -1847,7 +1883,7 @@ export function builtInAgentService(db: Db) { return state(definition, updated as Agent); } - async function reset(companyId: string, key: string, input: { resources?: Array<"agent" | "instructions" | "skill" | "routine"> } = {}) { + async function reset(companyId: string, key: string, input: { resources?: Array<"agent" | "instructions" | "skill" | "routine"> } = {}, actor?: AuthorizationActor) { const definition = requireBuiltInAgentDefinition(key); const resetAgentDefaults = !input.resources || input.resources.includes("agent"); const current = resetAgentDefaults @@ -1862,6 +1898,7 @@ export function builtInAgentService(db: Db) { definition, "reset", input.resources ? selectedBundleResources ?? [] : undefined, + actor, ); return state(definition, await agentSvc.getById(current.agent.id) as Agent, resources); } diff --git a/server/src/services/change-consent-gate.ts b/server/src/services/change-consent-gate.ts index 8523f04bcad..4569a0f8b38 100644 --- a/server/src/services/change-consent-gate.ts +++ b/server/src/services/change-consent-gate.ts @@ -112,7 +112,7 @@ function expandTargetKeysForLegacyCompatibility(targetKeys: string[]) { return [...expanded]; } -export function changeConsentGateService(db: Db) { +export function changeConsentGateService(db: Db | Parameters[0]>[0]) { return { assertConsented: async (input: { companyId: string; diff --git a/server/src/services/company-portability.ts b/server/src/services/company-portability.ts index 7c7501f6316..701894a7610 100644 --- a/server/src/services/company-portability.ts +++ b/server/src/services/company-portability.ts @@ -3549,7 +3549,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { const companies = companyService(db); const agents = agentService(db); const assetRecords = assetService(db); - const instructions = agentInstructionsService(); + const instructions = agentInstructionsService(db); const access = accessService(db); const projects = projectService(db); const issues = issueService(db); diff --git a/server/src/services/plugin-managed-agents.ts b/server/src/services/plugin-managed-agents.ts index 13473ec8c0a..c3a1bafd437 100644 --- a/server/src/services/plugin-managed-agents.ts +++ b/server/src/services/plugin-managed-agents.ts @@ -180,7 +180,7 @@ export function pluginManagedAgentService( ) { const agentSvc = agentService(db); const approvalSvc = approvalService(db); - const instructions = agentInstructionsService(); + const instructions = agentInstructionsService(db); function declarationFor(agentKey: string) { const declaration = options.manifest?.agents?.find((agent) => agent.agentKey === agentKey); diff --git a/ui/src/api/agents.ts b/ui/src/api/agents.ts index 5bde9d5e01d..b2050c0a42d 100644 --- a/ui/src/api/agents.ts +++ b/ui/src/api/agents.ts @@ -5,6 +5,9 @@ import type { AgentPermissions, AgentDetail, AgentInstructionsBundle, + AgentInstructionHistory, + AgentInstructionDiff, + AgentInstructionSnapshot, AgentInstructionsFileDetail, AgentSkillSnapshot, AdapterEnvironmentTestResult, @@ -167,9 +170,17 @@ export const agentsApi = { ), saveInstructionsFile: ( id: string, - data: { path: string; content: string; clearLegacyPromptTemplate?: boolean }, + data: { path: string; content: string; baseRevisionId?: string | null; clearLegacyPromptTemplate?: boolean }, companyId?: string, ) => api.put(agentPath(id, companyId, "/instructions-bundle/file"), data), + instructionHistory: (id: string, path: string, companyId?: string, cursor?: string) => + api.get(agentPath(id, companyId, `/instructions-bundle/history?path=${encodeURIComponent(path)}${cursor ? `&cursor=${encodeURIComponent(cursor)}` : ""}`)), + instructionRevision: (id: string, path: string, revisionId: string, companyId?: string) => + api.get(agentPath(id, companyId, `/instructions-bundle/revision/${revisionId}?path=${encodeURIComponent(path)}`)), + instructionDiff: (id: string, path: string, from: string, to: string, companyId?: string) => + api.get(agentPath(id, companyId, `/instructions-bundle/diff?path=${encodeURIComponent(path)}&from=${encodeURIComponent(from)}&to=${encodeURIComponent(to)}`)), + restoreInstructions: (id: string, data: { path: string; revisionId: string; baseRevisionId: string }, companyId?: string) => + api.post(agentPath(id, companyId, "/instructions-bundle/restore"), data), deleteInstructionsFile: (id: string, relativePath: string, companyId?: string) => api.delete( agentPath(id, companyId, `/instructions-bundle/file?path=${encodeURIComponent(relativePath)}`), diff --git a/ui/src/components/InstructionHistory.tsx b/ui/src/components/InstructionHistory.tsx new file mode 100644 index 00000000000..2d8dbecfdfe --- /dev/null +++ b/ui/src/components/InstructionHistory.tsx @@ -0,0 +1,145 @@ +import { useState } from "react"; +import { useInfiniteQuery, useMutation, useQuery } from "@tanstack/react-query"; +import type { AgentInstructionsFileDetail } from "@paperclipai/shared"; +import { agentsApi } from "../api/agents"; +import { Button } from "./ui/button"; + +export function InstructionHistory({ + agentId, + companyId, + path, + currentRevisionId, + disabled, + onRestored, +}: { + agentId: string; + companyId?: string; + path: string; + currentRevisionId: string; + disabled: boolean; + onRestored: (file: AgentInstructionsFileDetail) => void; +}) { + const [open, setOpen] = useState(false); + const [selected, setSelected] = useState(null); + const history = useInfiniteQuery({ + queryKey: ["instruction-history", agentId, path, currentRevisionId], + queryFn: ({ pageParam }) => + agentsApi.instructionHistory(agentId, path, companyId, pageParam), + initialPageParam: undefined as string | undefined, + getNextPageParam: (page) => page.nextCursor ?? undefined, + enabled: open, + }); + const diff = useQuery({ + queryKey: ["instruction-diff", agentId, path, selected, currentRevisionId], + queryFn: () => + agentsApi.instructionDiff( + agentId, + path, + selected!, + currentRevisionId, + companyId, + ), + enabled: open && Boolean(selected), + }); + const restore = useMutation({ + mutationFn: () => + agentsApi.restoreInstructions( + agentId, + { path, revisionId: selected!, baseRevisionId: currentRevisionId }, + companyId, + ), + onSuccess: (file) => { + setSelected(null); + onRestored(file); + }, + }); + const error = history.error ?? diff.error ?? restore.error; + return ( +
+ + {open && ( +
+ {history.isLoading && ( +

Loading revisions…

+ )} + {error && ( +

+ {error.message} +

+ )} + {history.data?.pages + .flatMap((page) => page.revisions) + .map((revision) => ( +
+ + + {revision.source} ·{" "} + {new Date(revision.createdAt).toLocaleString()} + {revision.id === currentRevisionId ? " · Current" : ""} + +
+ ))} + {history.hasNextPage && ( + + )} + {diff.data && ( + <> +

Selected revision

+
+                {diff.data.from.content}
+              
+

+ Changes from selected revision to current +

+
+                {diff.data.removed && `Removed:\n${diff.data.removed}\n`}
+                {diff.data.added && `Added:\n${diff.data.added}`}
+                {!diff.data.removed &&
+                  !diff.data.added &&
+                  "No content changes."}
+              
+ + {disabled && ( +

+ Save or cancel your edits before restoring. +

+ )} + + )} +
+ )} +
+ ); +} diff --git a/ui/src/pages/AgentDetail.instructions.test.tsx b/ui/src/pages/AgentDetail.instructions.test.tsx index bd33d7c435e..181da546841 100644 --- a/ui/src/pages/AgentDetail.instructions.test.tsx +++ b/ui/src/pages/AgentDetail.instructions.test.tsx @@ -14,6 +14,9 @@ const mockAgentsApi = vi.hoisted(() => ({ updateInstructionsBundle: vi.fn(), saveInstructionsFile: vi.fn(), deleteInstructionsFile: vi.fn(), + instructionHistory: vi.fn(), + instructionDiff: vi.fn(), + restoreInstructions: vi.fn(), })); const markdownEditorRenderMock = vi.hoisted(() => vi.fn()); @@ -333,6 +336,7 @@ describe("PromptsTab instruction editor", () => { { path: "AGENTS", content: "# Updated", + baseRevisionId: null, clearLegacyPromptTemplate: false, }, "company-1", @@ -340,6 +344,37 @@ describe("PromptsTab instruction editor", () => { }); }); + it("retains the draft and its base when a concurrent save conflicts", async () => { + const summary = makeSummary("AGENTS.md", "AGENTS.md"); + const revision = { id: "base-1", entryFile: "AGENTS.md" } as NonNullable; + await renderPromptsTab(makeBundle("AGENTS.md", [summary]), { "AGENTS.md": makeDetail(summary, "original", { revision }) }); + mockAgentsApi.saveInstructionsFile.mockRejectedValue(new Error("Instructions changed since the base revision")); + await selectInstructionMode("Edit"); + const editor = container.querySelector('[data-testid="markdown-editor"]')!; + await act(async () => { editor.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true })); setNativeValue(editor, "my unsaved edit"); }); + await waitFor(() => expect(saveAction).toEqual(expect.any(Function))); + await act(async () => { saveAction?.(); }); + await waitFor(() => expect(container.querySelector('[role="alert"]')?.textContent).toContain("base revision")); + expect(editor.value).toBe("my unsaved edit"); + expect(mockAgentsApi.saveInstructionsFile).toHaveBeenCalledWith("agent-1", expect.objectContaining({ content: "my unsaved edit", baseRevisionId: "base-1" }), "company-1"); + }); + + it("shows revision content and restores against the displayed current head", async () => { + const summary = makeSummary("AGENTS.md", "AGENTS.md"); + const revision = { id: "current-1", entryFile: "AGENTS.md" } as NonNullable; + mockAgentsApi.instructionHistory.mockResolvedValue({ revisions: [{ id: "old-revision", source: "board", createdAt: "2026-01-01T00:00:00Z" }], nextCursor: null }); + mockAgentsApi.instructionDiff.mockResolvedValue({ from: { content: "old text" }, removed: "old", added: "new" }); + mockAgentsApi.restoreInstructions.mockResolvedValue(makeDetail(summary, "old text", { revision: { ...revision, id: "restored-1" } })); + await renderPromptsTab(makeBundle("AGENTS.md", [summary]), { "AGENTS.md": makeDetail(summary, "new text", { revision }) }); + await waitFor(() => expect(buttonByText(container, "History")).toBeDefined()); + await act(async () => { buttonByText(container, "History").click(); }); + await waitFor(() => expect(buttonByText(container, "old-revi")).toBeDefined()); + await act(async () => { buttonByText(container, "old-revi").click(); }); + await waitFor(() => expect(container.textContent).toContain("old text")); + await act(async () => { buttonByText(container, "Restore as new revision").click(); }); + await waitFor(() => expect(mockAgentsApi.restoreInstructions).toHaveBeenCalledWith("agent-1", { path: "AGENTS.md", revisionId: "old-revision", baseRevisionId: "current-1" }, "company-1")); + }); + it("ignores rich-editor mount normalization until the user interacts", async () => { const summary = makeSummary("AGENTS.md", "AGENTS.md"); const onDirtyChange = vi.fn(); diff --git a/ui/src/pages/AgentDetail.tsx b/ui/src/pages/AgentDetail.tsx index b6896c6f729..b18b77bde49 100644 --- a/ui/src/pages/AgentDetail.tsx +++ b/ui/src/pages/AgentDetail.tsx @@ -1,3 +1,4 @@ +import { InstructionHistory } from "../components/InstructionHistory"; import { AgentCharacter } from "../components/AgentCharacter"; import { characterStateForAgent } from "@paperclipai/shared"; import { mergeRunLogChunks, readChunkSeq } from "../lib/run-log-chunks"; @@ -2197,6 +2198,7 @@ export function PromptsTab({ const [instructionMode, setInstructionMode] = useState<"read" | "edit" | "raw">("read"); const [showFilePanel, setShowFilePanel] = useState(false); const [draft, setDraft] = useState(null); + const draftBaseRevisionRef = useRef(undefined); const [bundleDraft, setBundleDraft] = useState<{ mode: "managed" | "external"; rootPath: string; @@ -2225,6 +2227,7 @@ export function PromptsTab({ }, []); const setSelectedFile = useCallback((filePath: string) => { editorInteractedRef.current = false; + draftBaseRevisionRef.current = undefined; setSelectedFileState(filePath); }, []); @@ -2284,7 +2287,7 @@ export function PromptsTab({ const selectedFileExists = bundleMatchesDraft && fileOptions.includes(selectedOrEntryFile); const selectedFileSummary = bundle?.files.find((file) => file.path === selectedOrEntryFile) ?? null; - const { data: selectedFileDetail, isLoading: fileLoading } = useQuery({ + const { data: selectedFileDetail, isLoading: fileLoading, error: fileError } = useQuery({ queryKey: queryKeys.agents.instructionsFile(agent.id, selectedOrEntryFile), queryFn: () => agentsApi.instructionsFile(agent.id, selectedOrEntryFile, companyId), enabled: Boolean(companyId && isLocal && selectedFileExists), @@ -2299,9 +2302,9 @@ export function PromptsTab({ }) => agentsApi.updateInstructionsBundle(agent.id, data, companyId), onMutate: () => { editorInteractedRef.current = false; - setAwaitingRefresh(true); }, onSuccess: () => { + setAwaitingRefresh(true); queryClient.invalidateQueries({ queryKey: queryKeys.agents.instructionsBundle(agent.id) }); queryClient.invalidateQueries({ queryKey: queryKeys.agents.detail(agent.id) }); queryClient.invalidateQueries({ queryKey: queryKeys.agents.detail(agent.urlKey) }); @@ -2310,13 +2313,15 @@ export function PromptsTab({ }); const saveFile = useMutation({ - mutationFn: (data: { path: string; content: string; clearLegacyPromptTemplate?: boolean }) => + mutationFn: (data: { path: string; content: string; baseRevisionId?: string | null; clearLegacyPromptTemplate?: boolean }) => agentsApi.saveInstructionsFile(agent.id, data, companyId), onMutate: () => { editorInteractedRef.current = false; - setAwaitingRefresh(true); }, - onSuccess: (_, variables) => { + onSuccess: (file, variables) => { + setDraft(null); + draftBaseRevisionRef.current = undefined; + queryClient.setQueryData(queryKeys.agents.instructionsFile(agent.id, variables.path), file); setPendingFiles((prev) => prev.filter((f) => f !== variables.path)); queryClient.invalidateQueries({ queryKey: queryKeys.agents.instructionsBundle(agent.id) }); queryClient.invalidateQueries({ queryKey: queryKeys.agents.instructionsFile(agent.id, variables.path) }); @@ -2409,7 +2414,7 @@ export function PromptsTab({ return; } if (lastFileVersionRef.current !== versionKey) { - setDraft(null); + if (draftBaseRevisionRef.current === undefined) setDraft(null); lastFileVersionRef.current = versionKey; } }, [awaitingRefresh, currentMode, currentRootPath, selectedFileDetail, selectedFileExists, selectedOrEntryFile]); @@ -2481,6 +2486,7 @@ export function PromptsTab({ await saveFile.mutateAsync({ path: selectedOrEntryFile, content: displayValue, + ...(selectedOrEntryFile === currentEntryFile && currentMode === "managed" ? { baseRevisionId: draftBaseRevisionRef.current !== undefined ? draftBaseRevisionRef.current : selectedFileDetail?.revision?.id ?? null } : {}), clearLegacyPromptTemplate: shouldClearLegacy, }); } @@ -2495,6 +2501,9 @@ export function PromptsTab({ fileDirty, isDirty, onSaveActionChange, + selectedFileDetail?.revision?.id, + currentEntryFile, + currentMode, saveFile, selectedOrEntryFile, updateBundle, @@ -2502,6 +2511,7 @@ export function PromptsTab({ useEffect(() => { onCancelActionChange(isDirty ? () => { + draftBaseRevisionRef.current = undefined; setDraft(null); if (bundle) { setBundleDraft({ @@ -2938,6 +2948,18 @@ export function PromptsTab({ + {(saveFile.error || fileError || updateBundle.error) &&

{(saveFile.error ?? fileError ?? updateBundle.error)?.message} Your unsaved edits are retained.

} + {selectedFileDetail?.receipt?.materialization === "pending" &&

Revision saved. The instruction file still needs to be rebuilt from the saved revision.

} + {selectedFileDetail?.revision && currentMode === "managed" && { + setDraft(null); + draftBaseRevisionRef.current = undefined; + queryClient.setQueryData(queryKeys.agents.instructionsFile(agent.id, selectedOrEntryFile), file); + queryClient.invalidateQueries({ queryKey: queryKeys.agents.instructionsBundle(agent.id) }); + }} + />} {selectedFileExists && fileLoading && !selectedFileDetail ? ( ) : instructionMode === "read" ? ( @@ -2975,6 +2997,7 @@ export function PromptsTab({ value={displayValue} onChange={(value) => { if (!editorInteractedRef.current) return; + if (draftBaseRevisionRef.current === undefined) draftBaseRevisionRef.current = selectedFileDetail?.revision?.id ?? null; setDraft(value ?? ""); }} placeholder="# Agent instructions" @@ -2991,7 +3014,10 @@ export function PromptsTab({