mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-17 20:09:05 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The Skills Manager gives operators a reusable skill layer, but iteration still required manual edits, ad hoc prompts, and indirect run inspection. > - Skill authors need a focused workflow for editing skill files, saving representative test inputs, and running those inputs through an agent without exposing harness tasks as normal company work. > - The backend therefore needs durable test inputs, reusable run templates, hidden harness issues, scoped run execution, retention metadata, and read-containment rules around hidden work. > - The frontend needs a three-pane Studio that keeps skill files, saved inputs/templates, and run output/history visible together while preserving the existing design system and token rules. > - This pull request ships that Skill Studio surface end to end: database migrations, shared contracts, server APIs/services, hidden harness execution behavior, UI routes/components, and focused tests. > - The benefit is faster and safer skill iteration, with inspectable outputs and fewer ways for internal harness work to leak into normal task lists, costs, or adjacent read APIs. ## Linked Issues or Issue Description No public GitHub issue exists for this feature. Feature request summary: - Problem: Skill authors need to edit and test company skills in one place instead of switching between the skill detail page, task creation, run output, and manual prompt history. - Proposed solution: Add a Skill Studio workbench with saved inputs, reusable templates, hidden sandboxed test runs, live run status, output inspection, run history, rerun/delete controls, and frontmatter-aware editing. - Expected users: Paperclip operators and agent-company maintainers who create, fork, import, and tune skills. - Related public PRs: Supersedes #9205, which was replaced so the public PR branch name follows contributor policy. - Duplicate search: searched public GitHub issues and PRs for "Skill Studio"; no other active public issue or PR directly covers this feature. ## What Changed - Added database migrations for Skill Studio test inputs, test runs, test run retention, and reusable run templates. - Added shared Skill Studio types, validators, route helpers, frontmatter utilities, and status handling. - Added server services and routes for saved inputs, test runs, templates, reruns, terminal-run deletion, hidden harness issue execution, and run-detail hydration. - Strengthened hidden-issue read containment across issue-adjacent routes and cost rollups used by skill test harness work. - Added the Skill Studio UI with skill file editing, frontmatter editing, saved inputs, templates, run creation/cancel/rerun/delete flows, output rendering, history, route support, and responsive pane behavior. - Added focused backend, shared, and UI tests for the new APIs, routing logic, editor/run behavior, hidden-issue containment, and migration safety. - Rebased onto current `master`, removed the generated lockfile diff from the PR, and verified no workflow files are changed. ## Verification - [x] `pnpm --filter @paperclipai/db check:migrations` - [x] `pnpm check:token-gates` - [x] `pnpm exec vitest run server/src/__tests__/company-skills-service.test.ts server/src/__tests__/company-skills-routes.test.ts server/src/__tests__/company-skill-test-runs-service.test.ts ui/src/lib/skill-studio.test.ts ui/src/pages/SkillStudio.test.tsx` — 5 files, 132 tests passed - [x] Greptile review on the latest PR head - [x] GitHub PR checks on the latest PR head ## Risks - Medium risk because this is a broad feature touching database schema, server orchestration, issue visibility, and a large UI surface. - Hidden harness issue containment is security-sensitive; this PR includes regression coverage for adjacent read paths and cost rollups. - The new migrations are additive and use idempotent guards where applicable, but deployed databases that previously tested draft migration numbers should still be checked carefully. - The UI depends on a new resizable panels package in `ui/package.json`; the lockfile is intentionally left to repository automation. > 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 Codex, GPT-5 coding agent with shell, git, and GitHub CLI tool use. Earlier feature commits include assistance from other Paperclip coding agents; this PR preparation, rebase, cleanup commit, and PR body were completed by OpenAI Codex in a Paperclip worktree. ## 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> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
229 lines
8.4 KiB
TypeScript
229 lines
8.4 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { flushSync } from "react-dom";
|
|
import { createRoot, type Root } from "react-dom/client";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { joinFrontmatterBlock, splitFrontmatterBlock } from "@paperclipai/shared";
|
|
import {
|
|
FrontmatterPanel,
|
|
type FrontmatterPanelChange,
|
|
type FrontmatterPanelProps,
|
|
} from "./FrontmatterPanel";
|
|
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
|
|
|
function flushUi(callback: () => void) {
|
|
flushSync(callback);
|
|
}
|
|
|
|
function setValue(el: HTMLInputElement | HTMLTextAreaElement, value: string) {
|
|
const proto = el instanceof HTMLTextAreaElement ? HTMLTextAreaElement.prototype : HTMLInputElement.prototype;
|
|
const setter = Object.getOwnPropertyDescriptor(proto, "value")!.set!;
|
|
flushUi(() => {
|
|
setter.call(el, value);
|
|
el.dispatchEvent(new Event("input", { bubbles: true }));
|
|
});
|
|
}
|
|
|
|
function click(el: Element | null | undefined) {
|
|
flushUi(() => {
|
|
el?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
|
|
});
|
|
}
|
|
|
|
function findButtonByText(container: HTMLElement, text: string): HTMLButtonElement | null {
|
|
return (
|
|
Array.from(container.querySelectorAll<HTMLButtonElement>("button")).find(
|
|
(button) => button.textContent?.trim() === text,
|
|
) ?? null
|
|
);
|
|
}
|
|
|
|
function expandPanel(container: HTMLElement) {
|
|
const toggle = container.querySelector<HTMLButtonElement>('button[aria-controls="frontmatter-panel-body"]');
|
|
click(toggle);
|
|
}
|
|
|
|
describe("FrontmatterPanel", () => {
|
|
let container: HTMLDivElement;
|
|
let root: Root;
|
|
|
|
beforeEach(() => {
|
|
container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
root = createRoot(container);
|
|
});
|
|
|
|
afterEach(() => {
|
|
flushUi(() => root.unmount());
|
|
container.remove();
|
|
});
|
|
|
|
function render(props: Partial<FrontmatterPanelProps> & { onChange?: (c: FrontmatterPanelChange) => void }) {
|
|
const onChange = props.onChange ?? vi.fn();
|
|
flushUi(() => {
|
|
root.render(
|
|
<FrontmatterPanel
|
|
frontmatterText={props.frontmatterText ?? ""}
|
|
hasFrontmatter={props.hasFrontmatter ?? false}
|
|
fileName={props.fileName ?? "SKILL.md"}
|
|
skillSlug={props.skillSlug}
|
|
readOnly={props.readOnly}
|
|
onChange={onChange}
|
|
/>,
|
|
);
|
|
});
|
|
return onChange;
|
|
}
|
|
|
|
it("does not emit on mount — byte-identity round-trip is preserved when untouched", () => {
|
|
const onChange = vi.fn();
|
|
render({
|
|
frontmatterText: "name: reflection-coach\ndescription: A helpful coach",
|
|
hasFrontmatter: true,
|
|
onChange,
|
|
});
|
|
expect(onChange).not.toHaveBeenCalled();
|
|
const toggle = container.querySelector<HTMLButtonElement>('button[aria-controls="frontmatter-panel-body"]');
|
|
expect(toggle?.getAttribute("aria-expanded")).toBe("false");
|
|
expect(container.querySelector("#fm-name")).toBeNull();
|
|
expandPanel(container);
|
|
// The Fields tab is available for this round-trippable block after expansion.
|
|
expect(container.querySelector<HTMLInputElement>("#fm-name")?.value).toBe("reflection-coach");
|
|
});
|
|
|
|
it("edits `name` in Fields mode and emits only that field changed", () => {
|
|
const onChange = vi.fn();
|
|
render({
|
|
frontmatterText: "name: coach\ndescription: A coach",
|
|
hasFrontmatter: true,
|
|
onChange,
|
|
});
|
|
expandPanel(container);
|
|
const nameInput = container.querySelector<HTMLInputElement>("#fm-name")!;
|
|
setValue(nameInput, "new-coach");
|
|
const last = onChange.mock.calls.at(-1)![0] as FrontmatterPanelChange;
|
|
expect(last.frontmatterText).toBe("name: new-coach\ndescription: A coach");
|
|
expect(last.hasFrontmatter).toBe(true);
|
|
});
|
|
|
|
it("edits allowed-tools via the chip input", () => {
|
|
const onChange = vi.fn();
|
|
render({
|
|
frontmatterText: "name: coach\nallowed-tools:\n - Read",
|
|
hasFrontmatter: true,
|
|
onChange,
|
|
});
|
|
expandPanel(container);
|
|
const chipInput = container.querySelector<HTMLInputElement>('input[aria-label="Add tool"]')!;
|
|
setValue(chipInput, "Grep");
|
|
flushUi(() => {
|
|
chipInput.dispatchEvent(
|
|
new KeyboardEvent("keydown", { key: "Enter", bubbles: true }),
|
|
);
|
|
});
|
|
const last = onChange.mock.calls.at(-1)![0] as FrontmatterPanelChange;
|
|
expect(last.frontmatterText).toBe("name: coach\nallowed-tools:\n - Read\n - Grep");
|
|
});
|
|
|
|
it("edits nested metadata scalar values", () => {
|
|
const onChange = vi.fn();
|
|
render({
|
|
frontmatterText: "name: coach\nmetadata:\n author: Paperclip\n version: 2",
|
|
hasFrontmatter: true,
|
|
onChange,
|
|
});
|
|
expandPanel(container);
|
|
const valueInput = container.querySelector<HTMLInputElement>('input[aria-label="Value for author"]')!;
|
|
setValue(valueInput, "Anthropic");
|
|
const last = onChange.mock.calls.at(-1)![0] as FrontmatterPanelChange;
|
|
// author changes; version stays a NUMBER (not requoted) because it was untouched.
|
|
expect(last.frontmatterText).toBe("name: coach\nmetadata:\n author: Anthropic\n version: 2");
|
|
});
|
|
|
|
it("locks Fields mode for non-round-trippable YAML (inline comment) and preserves raw bytes", () => {
|
|
const onChange = vi.fn();
|
|
const raw = "name: coach # keep me\ndescription: A coach";
|
|
render({ frontmatterText: raw, hasFrontmatter: true, onChange });
|
|
expandPanel(container);
|
|
|
|
// Fields tab is disabled; the YAML textarea is shown instead.
|
|
const fieldsTab = findButtonByText(container, "Fields");
|
|
expect(fieldsTab?.getAttribute("aria-disabled")).toBe("true");
|
|
const yaml = container.querySelector<HTMLTextAreaElement>('textarea[aria-label="Frontmatter YAML"]');
|
|
expect(yaml?.value).toBe(raw);
|
|
expect(container.querySelector("#fm-name")).toBeNull();
|
|
|
|
// Editing raw YAML passes through byte-for-byte.
|
|
setValue(yaml!, `${raw}\nextra: value`);
|
|
const last = onChange.mock.calls.at(-1)![0] as FrontmatterPanelChange;
|
|
expect(last.frontmatterText).toBe(`${raw}\nextra: value`);
|
|
});
|
|
|
|
it("shows a warning chip for missing name/description on SKILL.md", () => {
|
|
render({
|
|
frontmatterText: "name: coach",
|
|
hasFrontmatter: true,
|
|
fileName: "SKILL.md",
|
|
});
|
|
const chip = container.querySelector('[data-testid="frontmatter-warning-chip"]');
|
|
expect(chip?.textContent).toContain("issue");
|
|
});
|
|
|
|
it("flags a wrong-typed allowed-tools value instead of editing it as a list", () => {
|
|
render({
|
|
frontmatterText: "name: coach\ndescription: x\nallowed-tools: Read",
|
|
hasFrontmatter: true,
|
|
});
|
|
expandPanel(container);
|
|
expect(container.textContent).toContain("Expected a list");
|
|
});
|
|
|
|
it("adds frontmatter to a file that has none, seeding name from the slug", () => {
|
|
const onChange = vi.fn();
|
|
render({
|
|
frontmatterText: "",
|
|
hasFrontmatter: false,
|
|
fileName: "SKILL.md",
|
|
skillSlug: "reflection-coach",
|
|
onChange,
|
|
});
|
|
const addButton = container.querySelector<HTMLButtonElement>('[data-testid="add-frontmatter"]');
|
|
expect(addButton).toBeTruthy();
|
|
click(addButton);
|
|
const last = onChange.mock.calls.at(-1)![0] as FrontmatterPanelChange;
|
|
expect(last.hasFrontmatter).toBe(true);
|
|
expect(last.frontmatterText).toContain("name: reflection-coach");
|
|
expect(last.frontmatterText).toContain("description:");
|
|
});
|
|
|
|
it("renders read-only fields without editing affordances", () => {
|
|
render({
|
|
frontmatterText: "name: coach\nallowed-tools:\n - Read",
|
|
hasFrontmatter: true,
|
|
readOnly: true,
|
|
});
|
|
expandPanel(container);
|
|
const nameInput = container.querySelector<HTMLInputElement>("#fm-name");
|
|
expect(nameInput?.readOnly).toBe(true);
|
|
// No chip remove buttons, no add-tool input.
|
|
expect(container.querySelector('input[aria-label="Add tool"]')).toBeNull();
|
|
expect(container.querySelector('button[aria-label="Remove Read"]')).toBeNull();
|
|
});
|
|
|
|
it("keeps the full document byte-identical through split → panel → join with no edits", () => {
|
|
const file = "---\nname: coach\ndescription: A coach\n---\n# Body\n\nHello world\n";
|
|
const block = splitFrontmatterBlock(file);
|
|
const onChange = vi.fn();
|
|
render({
|
|
frontmatterText: block.frontmatterText,
|
|
hasFrontmatter: block.hasFrontmatter,
|
|
onChange,
|
|
});
|
|
expect(onChange).not.toHaveBeenCalled();
|
|
// The parent would rejoin the untouched block with the untouched body.
|
|
expect(joinFrontmatterBlock(block)).toBe(file);
|
|
});
|
|
});
|