fix(ui): use prose editor for markdown agent instructions (#9332)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent setup depends on instruction files that are readable and editable from the board UI > - The instructions tab already receives server-side metadata describing whether each file is Markdown > - The UI was deciding Markdown editor usage primarily from the file extension, which makes extensionless Markdown instruction files feel like raw code > - This pull request makes the instructions editor trust server Markdown metadata for existing files and keep extension fallback only for new unsaved files > - The benefit is that AGENTS-style prose instructions render and edit like prose while explicitly non-Markdown files still use the raw textarea ## Linked Issues or Issue Description - Refs #8201 - Refs #5652 - Refs #3427 - Refs #2068 - Related PRs: #2468, #2620 ## What Changed - Use server `markdown` metadata from instruction file details/summaries to choose the prose Markdown editor for existing instruction files. - Keep extension-based Markdown detection only for pending new files before server metadata exists. - Remove the monospace content styling from the Markdown editor path so prose instructions read like normal text. - Add focused tests for extensionless Markdown files, new `.md` files, and `.md` files explicitly marked non-Markdown by the server. ## Verification - `pnpm check:token-gates` - `pnpm exec vitest run ui/src/pages/AgentDetail.instructions.test.tsx` - `pnpm --filter @paperclipai/ui typecheck` ## Risks - Low risk. The editor selection now depends on server metadata for existing files, so incorrect server metadata would choose the wrong editor. The fallback still preserves extension-based behavior for newly created unsaved files. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI GPT-5 via Codex coding agent, tool-enabled terminal workflow. Context window details were not exposed by the runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
606aa4f266
commit
9a1d4b7983
|
|
@ -1470,7 +1470,7 @@ span.paperclip-mention-chip[data-mention-kind="external-object"] {
|
|||
--hex-eab308: #eab308; /* ActivityCharts.tsx — priority 'medium'; also the 0.5-0.8 success-rate bar tint. */
|
||||
--hex-6b7280: #6b7280; /* ActivityCharts.tsx — priority 'low' + unknown-status statusColors fallback. */
|
||||
--hex-10b981: #10b981; /* ActivityCharts.tsx — >=0.8 success-rate bar tint; also the run-activity 'succeeded' segment. */
|
||||
--hex-737373: #737373; /* ActivityCharts.tsx — run activity "other" segment tint. */
|
||||
--hex-737373: #737373; /* ActivityCharts.tsx — run activity 'other' segment tint. */
|
||||
--project-none: #64748b; /* Semantic rename of --hex-64748b (DECISION-SHEET.md A3, value unchanged). 'No project assigned' muted-slate fallback (TOKEN-AUDIT.md 1.3) across Routines/MarkdownEditor/RoutineRunVariablesDialog/RoutineList/IssueColumns/editable-sections; also ActivityCharts.tsx status 'backlog'. */
|
||||
--project-seed: #6366f1; /* Semantic rename of --hex-6366f1 (DECISION-SHEET.md A3, value unchanged). Project-color-fallback indigo seed default (ProjectDetail/PipelineSettings/IssueProperties/NewIssueDialog) — new-project-color-picker-seed family per TOKEN-AUDIT.md 1.3. */
|
||||
--liveness-blue: #2563eb; /* DECISION-SHEET.md A6: IssueChatThread.tsx human-message 'liveness blue' bubble (PAP-95 rev 5). Same value as --status-task-in_progress by coincidence, decoupled on purpose so a future status-hue change doesn't drag the chat bubble along. */
|
||||
|
|
|
|||
|
|
@ -0,0 +1,389 @@
|
|||
// @vitest-environment jsdom
|
||||
|
||||
import type { ComponentProps } from "react";
|
||||
import { flushSync } from "react-dom";
|
||||
import { createRoot, type Root } from "react-dom/client";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import type { Agent, AgentInstructionsBundle, AgentInstructionsFileDetail, AgentInstructionsFileSummary } from "@paperclipai/shared";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { PromptsTab } from "./AgentDetail";
|
||||
|
||||
const mockAgentsApi = vi.hoisted(() => ({
|
||||
instructionsBundle: vi.fn(),
|
||||
instructionsFile: vi.fn(),
|
||||
updateInstructionsBundle: vi.fn(),
|
||||
saveInstructionsFile: vi.fn(),
|
||||
deleteInstructionsFile: vi.fn(),
|
||||
}));
|
||||
|
||||
const markdownEditorRenderMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../api/agents", () => ({
|
||||
agentsApi: mockAgentsApi,
|
||||
}));
|
||||
|
||||
vi.mock("../api/assets", () => ({
|
||||
assetsApi: {
|
||||
uploadImage: vi.fn(async () => ({ contentPath: "/assets/uploaded-image.png" })),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("../context/CompanyContext", () => ({
|
||||
useCompany: () => ({ selectedCompanyId: "company-1" }),
|
||||
}));
|
||||
|
||||
vi.mock("../context/SidebarContext", () => ({
|
||||
useSidebar: () => ({ isMobile: false }),
|
||||
}));
|
||||
|
||||
vi.mock("@/adapters/use-adapter-capabilities", () => ({
|
||||
useAdapterCapabilities: () => () => ({
|
||||
supportsInstructionsBundle: true,
|
||||
supportsSkills: true,
|
||||
supportsLocalAgentJwt: true,
|
||||
requiresMaterializedRuntimeSkills: false,
|
||||
supportsModelProfiles: true,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../components/MarkdownEditor", () => ({
|
||||
MarkdownEditor: ({
|
||||
value,
|
||||
onChange,
|
||||
placeholder,
|
||||
contentClassName,
|
||||
imageUploadHandler,
|
||||
}: {
|
||||
value: string;
|
||||
onChange: (value: string) => void;
|
||||
placeholder?: string;
|
||||
contentClassName?: string;
|
||||
imageUploadHandler?: (file: File) => Promise<string>;
|
||||
}) => {
|
||||
markdownEditorRenderMock({
|
||||
value,
|
||||
contentClassName,
|
||||
hasImageUploadHandler: Boolean(imageUploadHandler),
|
||||
});
|
||||
return (
|
||||
<textarea
|
||||
data-testid="markdown-editor"
|
||||
aria-label="Markdown editor"
|
||||
className={contentClassName}
|
||||
placeholder={placeholder}
|
||||
value={value}
|
||||
onChange={(event) => onChange(event.target.value)}
|
||||
/>
|
||||
);
|
||||
},
|
||||
}));
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
async function act(callback: () => void | Promise<void>) {
|
||||
let result: void | Promise<void> = undefined;
|
||||
flushSync(() => {
|
||||
result = callback();
|
||||
});
|
||||
await result;
|
||||
}
|
||||
|
||||
async function flushReact() {
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
await new Promise((resolve) => window.setTimeout(resolve, 0));
|
||||
});
|
||||
}
|
||||
|
||||
async function waitFor<T>(assertion: () => T): Promise<T> {
|
||||
let lastError: unknown;
|
||||
for (let i = 0; i < 20; i++) {
|
||||
try {
|
||||
return assertion();
|
||||
} catch (error) {
|
||||
lastError = error;
|
||||
await flushReact();
|
||||
}
|
||||
}
|
||||
throw lastError;
|
||||
}
|
||||
|
||||
function setNativeValue(element: HTMLInputElement | HTMLTextAreaElement, value: string) {
|
||||
const prototype = element instanceof HTMLTextAreaElement
|
||||
? HTMLTextAreaElement.prototype
|
||||
: HTMLInputElement.prototype;
|
||||
const setter = Object.getOwnPropertyDescriptor(prototype, "value")?.set;
|
||||
setter?.call(element, value);
|
||||
element.dispatchEvent(new Event("input", { bubbles: true }));
|
||||
}
|
||||
|
||||
function buttonByText(container: HTMLElement, text: string) {
|
||||
const button = Array.from(container.querySelectorAll("button"))
|
||||
.find((candidate) => candidate.textContent?.trim() === text);
|
||||
if (!button) throw new Error(`Button not found: ${text}`);
|
||||
return button as HTMLButtonElement;
|
||||
}
|
||||
|
||||
function makeAgent(overrides: Partial<Agent> = {}): Agent {
|
||||
return {
|
||||
id: "agent-1",
|
||||
companyId: "company-1",
|
||||
name: "Codex Coder",
|
||||
urlKey: "codexcoder",
|
||||
role: "engineer",
|
||||
title: null,
|
||||
icon: null,
|
||||
status: "active",
|
||||
reportsTo: null,
|
||||
capabilities: null,
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
budgetMonthlyCents: 0,
|
||||
spentMonthlyCents: 0,
|
||||
pauseReason: null,
|
||||
pausedAt: null,
|
||||
permissions: { canCreateAgents: false },
|
||||
lastHeartbeatAt: null,
|
||||
metadata: null,
|
||||
createdAt: new Date("2026-01-01T00:00:00Z"),
|
||||
updatedAt: new Date("2026-01-01T00:00:00Z"),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeSummary(
|
||||
path: string,
|
||||
entryFile: string,
|
||||
overrides: Partial<AgentInstructionsFileSummary> = {},
|
||||
): AgentInstructionsFileSummary {
|
||||
const markdown = path.toLowerCase().endsWith(".md");
|
||||
return {
|
||||
path,
|
||||
size: 24,
|
||||
language: markdown ? "markdown" : "text",
|
||||
markdown,
|
||||
isEntryFile: path === entryFile,
|
||||
editable: true,
|
||||
deprecated: false,
|
||||
virtual: false,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeDetail(
|
||||
summary: AgentInstructionsFileSummary,
|
||||
content = "# Agent instructions",
|
||||
overrides: Partial<AgentInstructionsFileDetail> = {},
|
||||
): AgentInstructionsFileDetail {
|
||||
return {
|
||||
...summary,
|
||||
content,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeBundle(
|
||||
entryFile: string,
|
||||
files: AgentInstructionsFileSummary[],
|
||||
overrides: Partial<AgentInstructionsBundle> = {},
|
||||
): AgentInstructionsBundle {
|
||||
return {
|
||||
agentId: "agent-1",
|
||||
companyId: "company-1",
|
||||
mode: "managed",
|
||||
rootPath: "/paperclip/agents/agent-1/instructions",
|
||||
managedRootPath: "/paperclip/agents/agent-1/instructions",
|
||||
entryFile,
|
||||
resolvedEntryPath: `/paperclip/agents/agent-1/instructions/${entryFile}`,
|
||||
editable: true,
|
||||
warnings: [],
|
||||
legacyPromptTemplateActive: false,
|
||||
legacyBootstrapPromptTemplateActive: false,
|
||||
files,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("PromptsTab instruction editor", () => {
|
||||
let container: HTMLDivElement;
|
||||
let root: Root | null;
|
||||
let queryClient: QueryClient;
|
||||
let saveAction: (() => void) | null;
|
||||
|
||||
beforeEach(() => {
|
||||
container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
root = null;
|
||||
queryClient = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false }, mutations: { retry: false } },
|
||||
});
|
||||
saveAction = null;
|
||||
markdownEditorRenderMock.mockClear();
|
||||
Object.values(mockAgentsApi).forEach((mock) => mock.mockReset());
|
||||
mockAgentsApi.updateInstructionsBundle.mockResolvedValue({});
|
||||
mockAgentsApi.saveInstructionsFile.mockImplementation(async (_agentId, data) => ({
|
||||
path: data.path,
|
||||
size: data.content.length,
|
||||
language: "markdown",
|
||||
markdown: true,
|
||||
isEntryFile: true,
|
||||
editable: true,
|
||||
deprecated: false,
|
||||
virtual: false,
|
||||
content: data.content,
|
||||
}));
|
||||
mockAgentsApi.deleteInstructionsFile.mockResolvedValue({});
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
if (root) {
|
||||
await act(async () => {
|
||||
root?.unmount();
|
||||
});
|
||||
}
|
||||
queryClient.clear();
|
||||
container.remove();
|
||||
document.body.innerHTML = "";
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
async function renderPromptsTab(
|
||||
bundle: AgentInstructionsBundle,
|
||||
details: Record<string, AgentInstructionsFileDetail>,
|
||||
props: Partial<ComponentProps<typeof PromptsTab>> = {},
|
||||
) {
|
||||
mockAgentsApi.instructionsBundle.mockResolvedValue(bundle);
|
||||
mockAgentsApi.instructionsFile.mockImplementation(async (_agentId: string, path: string) => {
|
||||
const detail = details[path];
|
||||
if (!detail) throw new Error(`Missing detail for ${path}`);
|
||||
return detail;
|
||||
});
|
||||
|
||||
root = createRoot(container);
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<PromptsTab
|
||||
agent={props.agent ?? makeAgent()}
|
||||
companyId={props.companyId ?? "company-1"}
|
||||
onDirtyChange={props.onDirtyChange ?? vi.fn()}
|
||||
onSaveActionChange={props.onSaveActionChange ?? ((next) => { saveAction = next; })}
|
||||
onCancelActionChange={props.onCancelActionChange ?? vi.fn()}
|
||||
onSavingChange={props.onSavingChange ?? vi.fn()}
|
||||
/>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
await flushReact();
|
||||
}
|
||||
|
||||
it("uses server markdown metadata for extensionless files and saves MarkdownEditor drafts", async () => {
|
||||
const summary = makeSummary("AGENTS", "AGENTS", {
|
||||
language: "markdown",
|
||||
markdown: true,
|
||||
});
|
||||
await renderPromptsTab(
|
||||
makeBundle("AGENTS", [summary]),
|
||||
{ AGENTS: makeDetail(summary, "# Current") },
|
||||
);
|
||||
|
||||
const editor = await waitFor(() => {
|
||||
const candidate = container.querySelector<HTMLTextAreaElement>('[data-testid="markdown-editor"]');
|
||||
expect(candidate).not.toBeNull();
|
||||
return candidate!;
|
||||
});
|
||||
expect(markdownEditorRenderMock).toHaveBeenLastCalledWith(expect.objectContaining({
|
||||
contentClassName: expect.not.stringContaining("font-mono"),
|
||||
hasImageUploadHandler: true,
|
||||
value: "# Current",
|
||||
}));
|
||||
|
||||
await act(async () => {
|
||||
setNativeValue(editor, "# Updated");
|
||||
});
|
||||
await waitFor(() => {
|
||||
expect(saveAction).toEqual(expect.any(Function));
|
||||
});
|
||||
|
||||
saveAction?.();
|
||||
await waitFor(() => {
|
||||
expect(mockAgentsApi.saveInstructionsFile).toHaveBeenCalledWith(
|
||||
"agent-1",
|
||||
{
|
||||
path: "AGENTS",
|
||||
content: "# Updated",
|
||||
clearLegacyPromptTemplate: false,
|
||||
},
|
||||
"company-1",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("uses the Markdown editor for pending new .md files before server metadata exists", async () => {
|
||||
const summary = makeSummary("settings.json", "settings.json", {
|
||||
language: "json",
|
||||
markdown: false,
|
||||
});
|
||||
await renderPromptsTab(
|
||||
makeBundle("settings.json", [summary]),
|
||||
{ "settings.json": makeDetail(summary, "{\n \"ok\": true\n}") },
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(container.querySelector<HTMLTextAreaElement>('textarea[placeholder="File contents"]')).not.toBeNull();
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
buttonByText(container, "+").click();
|
||||
});
|
||||
await flushReact();
|
||||
const input = container.querySelector<HTMLInputElement>('input[placeholder="TOOLS.md"]');
|
||||
expect(input).not.toBeNull();
|
||||
|
||||
await act(async () => {
|
||||
setNativeValue(input!, "notes.md");
|
||||
buttonByText(container, "Create").click();
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(container.querySelector('[data-testid="markdown-editor"]')).not.toBeNull();
|
||||
});
|
||||
expect(mockAgentsApi.instructionsFile).not.toHaveBeenCalledWith("agent-1", "notes.md", "company-1");
|
||||
});
|
||||
|
||||
it("falls back to extension detection for existing .md files when metadata is missing", async () => {
|
||||
const summary = makeSummary("FALLBACK.md", "FALLBACK.md", {
|
||||
language: "text",
|
||||
markdown: undefined,
|
||||
});
|
||||
await renderPromptsTab(
|
||||
makeBundle("FALLBACK.md", [summary]),
|
||||
{ "FALLBACK.md": makeDetail(summary, "# Fallback", { markdown: undefined }) },
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(container.querySelector("[data-testid=\"markdown-editor\"]")).not.toBeNull();
|
||||
expect(markdownEditorRenderMock).toHaveBeenLastCalledWith(expect.objectContaining({
|
||||
value: "# Fallback",
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the raw textarea when server metadata marks an .md file as non-Markdown", async () => {
|
||||
const summary = makeSummary("NOTES.md", "NOTES.md", {
|
||||
language: "text",
|
||||
markdown: false,
|
||||
});
|
||||
await renderPromptsTab(
|
||||
makeBundle("NOTES.md", [summary]),
|
||||
{ "NOTES.md": makeDetail(summary, "raw instructions") },
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(container.querySelector('[data-testid="markdown-editor"]')).toBeNull();
|
||||
expect(container.querySelector<HTMLTextAreaElement>('textarea[placeholder="File contents"]')?.value).toBe("raw instructions");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -179,6 +179,17 @@ function isMarkdown(pathValue: string) {
|
|||
return pathValue.toLowerCase().endsWith(".md");
|
||||
}
|
||||
|
||||
function shouldUseMarkdownInstructionsEditor(input: {
|
||||
selectedFileExists: boolean;
|
||||
selectedPath: string;
|
||||
detail?: { markdown?: boolean } | null;
|
||||
summary?: { markdown?: boolean } | null;
|
||||
}) {
|
||||
const metadataMarkdown = input.detail?.markdown ?? input.summary?.markdown;
|
||||
if (typeof metadataMarkdown === "boolean") return metadataMarkdown;
|
||||
return isMarkdown(input.selectedPath);
|
||||
}
|
||||
|
||||
function formatEnvForDisplay(envValue: unknown, censorUsernameInLogs: boolean): string {
|
||||
const env = asRecord(envValue);
|
||||
if (!env) return "<unable-to-parse>";
|
||||
|
|
@ -1951,7 +1962,7 @@ function ConfigurationTab({
|
|||
|
||||
/* ---- Prompts Tab ---- */
|
||||
|
||||
function PromptsTab({
|
||||
export function PromptsTab({
|
||||
agent,
|
||||
companyId,
|
||||
onDirtyChange,
|
||||
|
|
@ -2190,6 +2201,12 @@ function PromptsTab({
|
|||
|
||||
const currentContent = selectedFileExists ? (selectedFileDetail?.content ?? "") : "";
|
||||
const displayValue = draft ?? currentContent;
|
||||
const useMarkdownEditor = shouldUseMarkdownInstructionsEditor({
|
||||
selectedFileExists,
|
||||
selectedPath: selectedOrEntryFile,
|
||||
detail: selectedFileDetail,
|
||||
summary: selectedFileSummary,
|
||||
});
|
||||
const bundleDirty = Boolean(
|
||||
bundleDraft &&
|
||||
(
|
||||
|
|
@ -2664,14 +2681,14 @@ function PromptsTab({
|
|||
|
||||
{selectedFileExists && fileLoading && !selectedFileDetail ? (
|
||||
<PromptEditorSkeleton />
|
||||
) : isMarkdown(selectedOrEntryFile) ? (
|
||||
) : useMarkdownEditor ? (
|
||||
<MarkdownEditor
|
||||
key={selectedOrEntryFile}
|
||||
value={displayValue}
|
||||
onChange={(value) => setDraft(value ?? "")}
|
||||
placeholder="# Agent instructions"
|
||||
className="min-w-0 overflow-hidden"
|
||||
contentClassName="min-h-(--sz-420px) max-w-full break-words text-sm font-mono"
|
||||
contentClassName="min-h-(--sz-420px) max-w-full break-words text-sm leading-7"
|
||||
imageUploadHandler={async (file) => {
|
||||
const namespace = `agents/${agent.id}/instructions/${selectedOrEntryFile.replaceAll("/", "-")}`;
|
||||
const asset = await uploadMarkdownImage.mutateAsync({ file, namespace });
|
||||
|
|
|
|||
Loading…
Reference in New Issue