From 154a4a7ac18d72680e7273653001fea69a2e8ed3 Mon Sep 17 00:00:00 2001 From: dotta Date: Wed, 18 Mar 2026 08:10:36 -0500 Subject: [PATCH] Improve instructions bundle mode switching Co-Authored-By: Paperclip --- packages/shared/src/types/agent.ts | 1 + .../agent-instructions-routes.test.ts | 2 + .../agent-instructions-service.test.ts | 123 +++++++++++++++ server/src/services/agent-instructions.ts | 28 ++++ ui/src/pages/AgentDetail.tsx | 146 +++++++++++++----- 5 files changed, 259 insertions(+), 41 deletions(-) create mode 100644 server/src/__tests__/agent-instructions-service.test.ts diff --git a/packages/shared/src/types/agent.ts b/packages/shared/src/types/agent.ts index e33a1fb4..61fe62fc 100644 --- a/packages/shared/src/types/agent.ts +++ b/packages/shared/src/types/agent.ts @@ -31,6 +31,7 @@ export interface AgentInstructionsBundle { companyId: string; mode: AgentInstructionsBundleMode | null; rootPath: string | null; + managedRootPath: string; entryFile: string; resolvedEntryPath: string | null; editable: boolean; diff --git a/server/src/__tests__/agent-instructions-routes.test.ts b/server/src/__tests__/agent-instructions-routes.test.ts index 3924fc04..4f6ca414 100644 --- a/server/src/__tests__/agent-instructions-routes.test.ts +++ b/server/src/__tests__/agent-instructions-routes.test.ts @@ -103,6 +103,7 @@ describe("agent instructions bundle routes", () => { companyId: "company-1", mode: "managed", rootPath: "/tmp/agent-1", + managedRootPath: "/tmp/agent-1", entryFile: "AGENTS.md", resolvedEntryPath: "/tmp/agent-1/AGENTS.md", editable: true, @@ -161,6 +162,7 @@ describe("agent instructions bundle routes", () => { expect(res.body).toMatchObject({ mode: "managed", rootPath: "/tmp/agent-1", + managedRootPath: "/tmp/agent-1", entryFile: "AGENTS.md", }); expect(mockAgentInstructionsService.getBundle).toHaveBeenCalled(); diff --git a/server/src/__tests__/agent-instructions-service.test.ts b/server/src/__tests__/agent-instructions-service.test.ts new file mode 100644 index 00000000..8129b5ec --- /dev/null +++ b/server/src/__tests__/agent-instructions-service.test.ts @@ -0,0 +1,123 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { agentInstructionsService } from "../services/agent-instructions.js"; + +type TestAgent = { + id: string; + companyId: string; + name: string; + adapterConfig: Record; +}; + +async function makeTempDir(prefix: string) { + return fs.mkdtemp(path.join(os.tmpdir(), prefix)); +} + +function makeAgent(adapterConfig: Record): TestAgent { + return { + id: "agent-1", + companyId: "company-1", + name: "Agent 1", + adapterConfig, + }; +} + +describe("agent instructions service", () => { + const originalPaperclipHome = process.env.PAPERCLIP_HOME; + const originalPaperclipInstanceId = process.env.PAPERCLIP_INSTANCE_ID; + const cleanupDirs = new Set(); + + afterEach(async () => { + if (originalPaperclipHome === undefined) delete process.env.PAPERCLIP_HOME; + else process.env.PAPERCLIP_HOME = originalPaperclipHome; + if (originalPaperclipInstanceId === undefined) delete process.env.PAPERCLIP_INSTANCE_ID; + else process.env.PAPERCLIP_INSTANCE_ID = originalPaperclipInstanceId; + + await Promise.all([...cleanupDirs].map(async (dir) => { + await fs.rm(dir, { recursive: true, force: true }); + cleanupDirs.delete(dir); + })); + }); + + it("copies the existing bundle into the managed root when switching to managed mode", async () => { + const paperclipHome = await makeTempDir("paperclip-agent-instructions-home-"); + const externalRoot = await makeTempDir("paperclip-agent-instructions-external-"); + cleanupDirs.add(paperclipHome); + cleanupDirs.add(externalRoot); + process.env.PAPERCLIP_HOME = paperclipHome; + process.env.PAPERCLIP_INSTANCE_ID = "test-instance"; + + await fs.writeFile(path.join(externalRoot, "AGENTS.md"), "# External Agent\n", "utf8"); + 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 agent = makeAgent({ + instructionsBundleMode: "external", + instructionsRootPath: externalRoot, + instructionsEntryFile: "AGENTS.md", + instructionsFilePath: path.join(externalRoot, "AGENTS.md"), + }); + + const result = await svc.updateBundle(agent, { mode: "managed" }); + + expect(result.bundle.mode).toBe("managed"); + expect(result.bundle.managedRootPath).toBe( + path.join( + paperclipHome, + "instances", + "test-instance", + "companies", + "company-1", + "agents", + "agent-1", + "instructions", + ), + ); + expect(result.bundle.files.map((file) => file.path)).toEqual(["AGENTS.md", "docs/TOOLS.md"]); + await expect(fs.readFile(path.join(result.bundle.managedRootPath, "AGENTS.md"), "utf8")).resolves.toBe("# External Agent\n"); + await expect(fs.readFile(path.join(result.bundle.managedRootPath, "docs", "TOOLS.md"), "utf8")).resolves.toBe("## Tools\n"); + }); + + it("creates the target entry file when switching to a new external root", async () => { + const paperclipHome = await makeTempDir("paperclip-agent-instructions-home-"); + const managedRoot = path.join( + paperclipHome, + "instances", + "test-instance", + "companies", + "company-1", + "agents", + "agent-1", + "instructions", + ); + const externalRoot = await makeTempDir("paperclip-agent-instructions-new-external-"); + cleanupDirs.add(paperclipHome); + cleanupDirs.add(externalRoot); + process.env.PAPERCLIP_HOME = paperclipHome; + process.env.PAPERCLIP_INSTANCE_ID = "test-instance"; + + await fs.mkdir(managedRoot, { recursive: true }); + await fs.writeFile(path.join(managedRoot, "AGENTS.md"), "# Managed Agent\n", "utf8"); + + const svc = agentInstructionsService(); + const agent = makeAgent({ + instructionsBundleMode: "managed", + instructionsRootPath: managedRoot, + instructionsEntryFile: "AGENTS.md", + instructionsFilePath: path.join(managedRoot, "AGENTS.md"), + }); + + const result = await svc.updateBundle(agent, { + mode: "external", + rootPath: externalRoot, + entryFile: "docs/AGENTS.md", + }); + + expect(result.bundle.mode).toBe("external"); + expect(result.bundle.rootPath).toBe(externalRoot); + await expect(fs.readFile(path.join(externalRoot, "docs", "AGENTS.md"), "utf8")).resolves.toBe("# Managed Agent\n"); + }); +}); diff --git a/server/src/services/agent-instructions.ts b/server/src/services/agent-instructions.ts index 197fe960..086d5fbb 100644 --- a/server/src/services/agent-instructions.ts +++ b/server/src/services/agent-instructions.ts @@ -42,6 +42,7 @@ type AgentInstructionsBundle = { companyId: string; mode: BundleMode | null; rootPath: string | null; + managedRootPath: string; entryFile: string; resolvedEntryPath: string | null; editable: boolean; @@ -266,6 +267,7 @@ function toBundle(agent: AgentLike, state: BundleState, files: AgentInstructions companyId: agent.companyId, mode: state.mode, rootPath: state.rootPath, + managedRootPath: resolveManagedInstructionsRoot(agent), entryFile: state.entryFile, resolvedEntryPath: state.resolvedEntryPath, editable: Boolean(state.rootPath), @@ -299,6 +301,21 @@ function applyBundleConfig( return next; } +async function writeBundleFiles( + rootPath: string, + files: Record, + options?: { overwriteExisting?: boolean }, +) { + for (const [relativePath, content] of Object.entries(files)) { + const normalizedPath = normalizeRelativeFilePath(relativePath); + const absolutePath = resolvePathWithinRoot(rootPath, normalizedPath); + const existingStat = await statIfExists(absolutePath); + if (existingStat?.isFile() && !options?.overwriteExisting) continue; + await fs.mkdir(path.dirname(absolutePath), { recursive: true }); + await fs.writeFile(absolutePath, content, "utf8"); + } +} + export function syncInstructionsBundleConfigFromFilePath( agent: AgentLike, adapterConfig: Record, @@ -440,6 +457,17 @@ export function agentInstructionsService() { await fs.mkdir(nextRootPath, { recursive: true }); + const existingFiles = await listFilesRecursive(nextRootPath); + const exported = await exportFiles(agent); + if (existingFiles.length === 0) { + await writeBundleFiles(nextRootPath, exported.files); + } + const refreshedFiles = existingFiles.length === 0 ? await listFilesRecursive(nextRootPath) : existingFiles; + if (!refreshedFiles.includes(nextEntryFile)) { + const nextEntryContent = exported.files[nextEntryFile] ?? exported.files[exported.entryFile] ?? ""; + await writeBundleFiles(nextRootPath, { [nextEntryFile]: nextEntryContent }); + } + const nextConfig = applyBundleConfig(state.config, { mode: nextMode, rootPath: nextRootPath, diff --git a/ui/src/pages/AgentDetail.tsx b/ui/src/pages/AgentDetail.tsx index 22823e99..736ab876 100644 --- a/ui/src/pages/AgentDetail.tsx +++ b/ui/src/pages/AgentDetail.tsx @@ -1513,6 +1513,11 @@ function PromptsTab({ const containerRef = useRef(null); const [awaitingRefresh, setAwaitingRefresh] = useState(false); const lastFileVersionRef = useRef(null); + const externalBundleRef = useRef<{ + rootPath: string; + entryFile: string; + selectedFile: string; + } | null>(null); const isLocal = agent.adapterType === "claude_local" || @@ -1528,23 +1533,35 @@ function PromptsTab({ enabled: Boolean(companyId && isLocal), }); - const currentMode = bundleDraft?.mode ?? bundle?.mode ?? "managed"; + const persistedMode = bundle?.mode ?? "managed"; + const persistedRootPath = persistedMode === "managed" + ? (bundle?.managedRootPath ?? bundle?.rootPath ?? "") + : (bundle?.rootPath ?? ""); + const currentMode = bundleDraft?.mode ?? persistedMode; const currentEntryFile = bundleDraft?.entryFile ?? bundle?.entryFile ?? "AGENTS.md"; - const currentRootPath = bundleDraft?.rootPath ?? bundle?.rootPath ?? ""; + const currentRootPath = bundleDraft?.rootPath ?? persistedRootPath; const fileOptions = useMemo( () => bundle?.files.map((file) => file.path) ?? [], [bundle], ); + const bundleMatchesDraft = Boolean( + bundle && + currentMode === persistedMode && + currentEntryFile === bundle.entryFile && + currentRootPath === persistedRootPath, + ); const visibleFilePaths = useMemo( - () => [...new Set([currentEntryFile, ...fileOptions])], - [currentEntryFile, fileOptions], + () => bundleMatchesDraft + ? [...new Set([currentEntryFile, ...fileOptions])] + : [currentEntryFile], + [bundleMatchesDraft, currentEntryFile, fileOptions], ); const fileTree = useMemo( () => buildFileTree(Object.fromEntries(visibleFilePaths.map((filePath) => [filePath, ""]))), [visibleFilePaths], ); const selectedOrEntryFile = selectedFile || currentEntryFile; - const selectedFileExists = fileOptions.includes(selectedOrEntryFile); + const selectedFileExists = bundleMatchesDraft && fileOptions.includes(selectedOrEntryFile); const selectedFileSummary = bundle?.files.find((file) => file.path === selectedOrEntryFile) ?? null; const { data: selectedFileDetail, isLoading: fileLoading } = useQuery({ @@ -1603,6 +1620,10 @@ function PromptsTab({ useEffect(() => { if (!bundle) return; + if (!bundleMatchesDraft) { + if (selectedFile !== currentEntryFile) setSelectedFile(currentEntryFile); + return; + } const availablePaths = bundle.files.map((file) => file.path); if (availablePaths.length === 0) { if (selectedFile !== bundle.entryFile) setSelectedFile(bundle.entryFile); @@ -1611,7 +1632,7 @@ function PromptsTab({ if (!availablePaths.includes(selectedFile)) { setSelectedFile(availablePaths.includes(bundle.entryFile) ? bundle.entryFile : availablePaths[0]!); } - }, [bundle, selectedFile]); + }, [bundle, bundleMatchesDraft, currentEntryFile, selectedFile]); useEffect(() => { const nextExpanded = new Set(); @@ -1627,7 +1648,9 @@ function PromptsTab({ }, [visibleFilePaths]); useEffect(() => { - const versionKey = selectedFileDetail ? `${selectedFileDetail.path}:${selectedFileDetail.content}` : `draft:${selectedOrEntryFile}`; + const versionKey = selectedFileExists && selectedFileDetail + ? `${selectedFileDetail.path}:${selectedFileDetail.content}` + : `draft:${currentMode}:${currentRootPath}:${selectedOrEntryFile}`; if (awaitingRefresh) { setAwaitingRefresh(false); setBundleDraft(null); @@ -1639,27 +1662,36 @@ function PromptsTab({ setDraft(null); lastFileVersionRef.current = versionKey; } - }, [awaitingRefresh, selectedFileDetail, selectedOrEntryFile]); + }, [awaitingRefresh, currentMode, currentRootPath, selectedFileDetail, selectedFileExists, selectedOrEntryFile]); useEffect(() => { if (!bundle) return; setBundleDraft((current) => { if (current) return current; return { - mode: bundle.mode ?? "managed", - rootPath: bundle.rootPath ?? "", + mode: persistedMode, + rootPath: persistedRootPath, entryFile: bundle.entryFile, }; }); - }, [bundle]); + }, [bundle, persistedMode, persistedRootPath]); + + useEffect(() => { + if (!bundle || currentMode !== "external") return; + externalBundleRef.current = { + rootPath: currentRootPath, + entryFile: currentEntryFile, + selectedFile: selectedOrEntryFile, + }; + }, [bundle, currentEntryFile, currentMode, currentRootPath, selectedOrEntryFile]); const currentContent = selectedFileExists ? (selectedFileDetail?.content ?? "") : ""; const displayValue = draft ?? currentContent; const bundleDirty = Boolean( bundleDraft && ( - bundleDraft.mode !== (bundle?.mode ?? "managed") || - bundleDraft.rootPath !== (bundle?.rootPath ?? "") || + bundleDraft.mode !== persistedMode || + bundleDraft.rootPath !== persistedRootPath || bundleDraft.entryFile !== (bundle?.entryFile ?? "AGENTS.md") ), ); @@ -1710,13 +1742,13 @@ function PromptsTab({ setDraft(null); if (bundle) { setBundleDraft({ - mode: bundle.mode ?? "managed", - rootPath: bundle.rootPath ?? "", + mode: persistedMode, + rootPath: persistedRootPath, entryFile: bundle.entryFile, }); } } : null); - }, [bundle, isDirty, onCancelActionChange]); + }, [bundle, isDirty, onCancelActionChange, persistedMode, persistedRootPath]); const handleSeparatorDrag = useCallback((event: React.MouseEvent) => { event.preventDefault(); @@ -1754,7 +1786,7 @@ function PromptsTab({ } return ( -
+
{(bundle?.warnings ?? []).length > 0 && (
{(bundle?.warnings ?? []).map((warning) => ( @@ -1907,7 +1939,7 @@ function PromptsTab({ Advanced - +
-
+