From 7fd05e1354acd6e08e3a9de2edaaec872fcbf261 Mon Sep 17 00:00:00 2001
From: Dotta <34892728+cryppadotta@users.noreply.github.com>
Date: Thu, 5 Nov 2026 07:58:34 -0600
Subject: [PATCH] fix(skills): keep initial file editor drafts clean (#19241)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - People inspect and edit skill files from the Skills library.
> - The Markdown editor receives the body without YAML frontmatter.
> - The skill page compared that body with the entire saved file.
> - Opening Edit therefore showed unsaved changes before the person
typed.
> - This pull request uses the same saved body for initialization,
comparison and Discard.
> - Clean files stay clean, while real edits retain the existing save
path.

## Linked Issues or Issue Description

**What happened?**

Open an editable SKILL.md with YAML frontmatter in the Skills library.
Select Files, then Edit. The page immediately shows Unsaved changes
without input. The draft contains only the body, while the dirty check
compares it with the full file.

**Expected behavior**

Opening Edit leaves the file clean and Save disabled. Real input enables
Save. Discard restores the original body. Saving an edit keeps the
existing frontmatter.

**Steps to reproduce**

1. Open a managed skill whose SKILL.md has YAML frontmatter and a
Markdown body.
2. Select Files, then Edit.
3. Make no editor input.
4. Observe the dirty indicator and Save state.

**Paperclip version or commit**

Reproduced locally on master e8163fc86024548ade7cad7f6df42f9e0c25f242.

**Deployment mode**

Rendered local page tests for both Skills-library interface variants.
API data and the rich-editor boundary are mocked. Fixed live-browser
acceptance remains unverified.

Related: #13915 suppresses initial rich-editor normalization, and #9356
improves Skill Studio edit tracking. This defect occurs in the
Skills-library page before any editor change event.

## What Changed

- Compare the draft with the same body used by Markdown initialization.
- Restore that body on Discard, without inserting frontmatter into the
body editor.
- Disable Save when the file is clean.
- Cover both page variants through actual page hydration and the Edit
button.
- Preserve real edits, exact fixture frontmatter, custom metadata and
file-selection behavior.

## Verification

- Before the fix, both rendered page variants fail after Edit shows
Unsaved changes without an editor change event.
- The same regressions pass after the four-line production correction.
- 137 focused tests pass across CompanySkills, MarkdownEditor and
company-skill-routes.
- The page regression checks genuine input, Discard and re-entry, an
explicit metadata-preserving save, and switching away from an unsaved
prior-file draft without another save.
- UI typecheck and production build pass. Token gates and diff checks
pass.
- The first typecheck lacked ignored generated Storybook catalog files.
The existing generator supplied them; the failed prerequisite attempt
remains recorded.
- Independent local Sol review passed the candidate.
- Full-workspace LOCAL typecheck, tests and build were not run.
- Hosted CI passes on head 8336145: typecheck, build, selected general
tests, all eight E2E shards and aggregate gates. Four scope-based checks
are skipped: Storybook visual, Storybook deployment, Runner matrix and
serialized server suites. This is affected-scope coverage.
- Greptile reviewed the exact head with a 5/5 score and no actionable
findings.
- Paperclip native review was filtered by the configured Draft and Ready
event policy. This is not a completed native review or a native 5/5
result. The review route uses independent local Sol review and hosted
Greptile. Repository-required checks and reviews still govern merge.
- No fixed live-browser, deployed behavior or canonical-file mutation is
claimed.

## Risks

The change affects the Skills-library page baseline in both existing
interface variants. It keeps the existing frontmatter merge and
file-write API. Rich-editor normalization, Skill Studio, settings,
permissions and selection lifecycle remain unchanged. The tests isolate
the page baseline with a controlled editor boundary; they do not prove a
live rich-editor or server write journey.

## Model Used

OpenAI gpt-6.1-sol through Codex, with reasoning, source inspection,
code execution and local tests. The runtime did not expose
context-window size.

## 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 <paperclip@paperclipai.com>
---
 ui/src/pages/CompanySkills.production.tsx |  4 +-
 ui/src/pages/CompanySkills.test.tsx       | 66 +++++++++++++++++++++--
 ui/src/pages/CompanySkills.tsx            |  4 +-
 3 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/ui/src/pages/CompanySkills.production.tsx b/ui/src/pages/CompanySkills.production.tsx
index cc0ebc79c10..1579df556fc 100644
--- a/ui/src/pages/CompanySkills.production.tsx
+++ b/ui/src/pages/CompanySkills.production.tsx
@@ -2987,7 +2987,7 @@ export function SkillDetailPage({
 
   // Track unsaved edits so we can float a save bar and warn before the page is
   // unloaded with a dirty draft (PAP-10907 J).
-  const savedFileContent = file?.content ?? "";
+  const savedFileContent = file?.markdown ? splitFrontmatter(file.content).body : file?.content ?? "";
   const isDirty = editMode && Boolean(file?.editable) && draft !== savedFileContent;
   useEffect(() => {
     if (!isDirty) return;
@@ -3086,7 +3086,7 @@ export function SkillDetailPage({
                 editMode ? (
                   <>
                     <Button variant="ghost" size="sm" onClick={() => setEditMode(false)} disabled={savePending}>Cancel</Button>
-                    <Button size="sm" onClick={onSave} disabled={savePending}>
+                    <Button size="sm" onClick={onSave} disabled={savePending || !isDirty}>
                       <Save className="mr-1.5 h-3.5 w-3.5" />
                       {savePending ? "Saving..." : "Save"}
                     </Button>
diff --git a/ui/src/pages/CompanySkills.test.tsx b/ui/src/pages/CompanySkills.test.tsx
index dc9798d06b5..f10fcc72e88 100644
--- a/ui/src/pages/CompanySkills.test.tsx
+++ b/ui/src/pages/CompanySkills.test.tsx
@@ -23,6 +23,7 @@ import { CompanySkills as ProductionCompanySkills } from "./CompanySkills.produc
 import { queryKeys } from "../lib/queryKeys";
 
 const readVersion = vi.hoisted(() => vi.fn());
+const editorRoute = vi.hoisted(() => ({ path: undefined as string | undefined, search: "" }));
 const mockCompanySkillsApi = vi.hoisted(() => ({
   version: readVersion,
   list: vi.fn(),
@@ -34,6 +35,7 @@ const mockCompanySkillsApi = vi.hoisted(() => ({
   catalogDetail: vi.fn(),
   catalogFile: vi.fn(),
   browseProject: vi.fn(),
+  updateFile: vi.fn(),
 }));
 const mockFoldersApi = vi.hoisted(() => ({ list: vi.fn() }));
 const mockAgentsApi = vi.hoisted(() => ({ list: vi.fn() }));
@@ -63,8 +65,8 @@ vi.mock("@/lib/router", () => ({
     <a href={to} {...props}>{children}</a>
   ),
   useNavigate: () => vi.fn(),
-  useParams: () => ({}),
-  useSearchParams: () => [new URLSearchParams(), vi.fn()],
+  useParams: () => ({ "*": editorRoute.path }),
+  useSearchParams: () => [new URLSearchParams(editorRoute.search), vi.fn()],
 }));
 
 vi.mock("@/components/ui/button", () => ({
@@ -134,7 +136,7 @@ vi.mock("../components/MarkdownBody", () => ({
 }));
 
 vi.mock("../components/MarkdownEditor", () => ({
-  MarkdownEditor: ({ value }: { value: string }) => <textarea readOnly value={value} />,
+  MarkdownEditor: ({ value, onChange }: { value: string; onChange: (value: string) => void }) => <textarea aria-label="Skill markdown" value={value} onInput={event => onChange(event.currentTarget.value)} onChange={() => {}} />,
 }));
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -151,6 +153,8 @@ async function act(callback: () => void | Promise<void>) {
 }
 
 afterEach(() => {
+  editorRoute.path = undefined;
+  editorRoute.search = "";
   root?.unmount();
   root = null;
   queryClient?.clear();
@@ -1278,6 +1282,62 @@ describe("CompanySkills page read resilience", () => {
     vi.unstubAllGlobals();
   });
 
+  describe.each([["current", CompanySkills], ["production", ProductionCompanySkills]] as const)("%s file editor baseline", (_name, Page) => {
+    it("opens frontmatter markdown clean without an editor change, preserves edits and resets selection", async () => {
+      const frontmatter = "---\nname: demo-skill\ndescription: Keep this metadata.\ncustom: stable\n---\n\n";
+      const body = "# Demo Skill\n\nOriginal body.\n";
+      const content = frontmatter + body;
+      const detail = makeDetail(makeVersion(1, content));
+      detail.fileInventory.push({ path: "references/notes.md", kind: "reference" });
+      editorRoute.path = "demo-skill";
+      editorRoute.search = "tab=files";
+      const files: Record<string, string> = { "SKILL.md": content, "references/notes.md": "Reference body.\n" };
+      const readFile = (path: string) => ({ path, content: files[path], markdown: true, editable: true, encoding: "utf-8" });
+      mockCompanySkillsApi.detail.mockResolvedValue(detail);
+      mockCompanySkillsApi.file.mockImplementation(async (_company, _skill, path) => readFile(path));
+      mockCompanySkillsApi.versions.mockResolvedValue([]);
+      mockCompanySkillsApi.updateStatus.mockResolvedValue({ supported: false });
+      mockCompanySkillsApi.updateFile.mockReset().mockImplementation(async (_company, _skill, path, next) => {
+        files[path] = next;
+        return readFile(path);
+      });
+      const node = await renderPage(Page);
+      await vi.waitFor(() => expect(buttonsNamed(node, "Edit")).toHaveLength(1));
+      await click(buttonsNamed(node, "Edit")[0]!);
+      expect(node.textContent).not.toContain("Unsaved changes");
+      expect((buttonsNamed(node, "Save")[0] as HTMLButtonElement).disabled).toBe(true);
+      const editor = () => node.querySelector<HTMLTextAreaElement>('[aria-label="Skill markdown"]')!;
+      expect(editor().value).toBe(body);
+      expect(mockCompanySkillsApi.updateFile).not.toHaveBeenCalled();
+      const change = async (value: string) => act(() => {
+        Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value")!.set!.call(editor(), value);
+        editor().dispatchEvent(new Event("input", { bubbles: true }));
+      });
+      await change(body + "A real edit.\n");
+      expect(node.textContent).toContain("Unsaved changes");
+      expect((buttonsNamed(node, "Save")[0] as HTMLButtonElement).disabled).toBe(false);
+      await click(buttonsNamed(node, "Discard")[0]!);
+      await click(buttonsNamed(node, "Edit")[0]!);
+      expect(editor().value).toBe(body);
+      expect(node.textContent).not.toContain("Unsaved changes");
+      await change(body + "Saved edit.\n");
+      await click(buttonsNamed(node, "Save")[0]!);
+      await vi.waitFor(() => expect(mockCompanySkillsApi.updateFile).toHaveBeenCalledWith("company-1", "skill-1", "SKILL.md", frontmatter + body + "Saved edit.\n"));
+      expect(detail.description).toBe("A demo skill.");
+      await settle();
+      await click(buttonsNamed(node, "Edit")[0]!);
+      await change(body + "Unsaved prior-file edit.\n");
+      editorRoute.path = "demo-skill/files/references/notes.md";
+      await act(() => root?.render(<QueryClientProvider client={queryClient}><Page /></QueryClientProvider>));
+      await settle();
+      await click(buttonsNamed(node, "Edit")[0]!);
+      expect(editor().value).toBe("Reference body.\n");
+      expect(node.textContent).not.toContain("Unsaved changes");
+      expect((buttonsNamed(node, "Save")[0] as HTMLButtonElement).disabled).toBe(true);
+      expect(mockCompanySkillsApi.updateFile).toHaveBeenCalledTimes(1);
+    });
+  });
+
   describe.each([["current", CompanySkills], ["production", ProductionCompanySkills]] as const)("%s discovery errors", (_name, Page) => {
     it.each([["skills", 403], ["skills", 429], ["catalog", 403], ["catalog", 429]] as const)(
       "shows a %s %s failure while the other read pauses and keeps its own Retry usable",
diff --git a/ui/src/pages/CompanySkills.tsx b/ui/src/pages/CompanySkills.tsx
index f9fb4d68473..05e68aa9786 100644
--- a/ui/src/pages/CompanySkills.tsx
+++ b/ui/src/pages/CompanySkills.tsx
@@ -2897,7 +2897,7 @@ export function SkillDetailPage({
 
   // Track unsaved edits so we can float a save bar and warn before the page is
   // unloaded with a dirty draft (PAP-10907 J).
-  const savedFileContent = file?.content ?? "";
+  const savedFileContent = file?.markdown ? splitFrontmatter(file.content).body : file?.content ?? "";
   const isDirty = editMode && Boolean(file?.editable) && draft !== savedFileContent;
   useEffect(() => {
     if (!isDirty) return;
@@ -3001,7 +3001,7 @@ export function SkillDetailPage({
                 editMode ? (
                   <>
                     <Button variant="ghost" size="sm" onClick={() => setEditMode(false)} disabled={savePending}>Cancel</Button>
-                    <Button size="sm" onClick={onSave} disabled={savePending}>
+                    <Button size="sm" onClick={onSave} disabled={savePending || !isDirty}>
                       <Save className="mr-1.5 h-3.5 w-3.5" />
                       {savePending ? "Saving..." : "Save"}
                     </Button>