mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 10:13:11 +02:00
## Thinking Path > - Paperclip is the open source control plane people use to organize and govern AI-agent companies > - Company skills are durable resources that users browse, import, assign, and maintain over time > - A flat skill list plus tags does not provide a stable location or hierarchy for personal, company, project-imported, and bundled skills > - Folder paths need to be canonical, company-scoped, safe to move, and preserved across re-imports without changing skill IDs > - The `/skills` UI also needs traversal, breadcrumbs, move/create flows, and a dedicated My Skills namespace that work on desktop and mobile > - This pull request adds the folder data model and APIs, reserved-root lifecycle, project import behavior, and the folder-first skills experience > - The benefit is a predictable filesystem-like organization model while tags remain available for cross-cutting classification ## Linked Issues or Issue Description Refs #9619 — the reviewed folder foundation was intentionally closed and folded into this combined feature PR. Refs #9026 — earlier flat-folder attempt superseded by this integrated implementation. Refs #3281 — related skill organization proposal; this PR uses canonical persisted folders rather than deriving groups from skill keys, and does not add hidden-skill behavior. **Feature request** - **Problem:** Skills currently lack a canonical hierarchical location, making personal skills, project imports, bundled skills, and company-authored skills difficult to traverse and manage at scale. - **Proposed behavior:** Add nested company-scoped folders with stable paths, reserved My/Projects/Bundled roots, subtree queries, safe move/create operations, and a folder-first `/skills` library UI. - **Import behavior:** New project scans file skills under `projects/<project-slug>`; later imports update content without overriding a user-selected folder. - **Alternatives considered:** Tags alone remain useful for cross-cutting classification, but they do not provide canonical location, nesting, reserved namespaces, or stable import placement. - **Roadmap alignment:** Extends the completed Skills Manager and Scheduled Routines capabilities without duplicating an active roadmap item. ## What Changed - Adds `folders` persistence for routine and skill folders, nested canonical paths, parent/slug/system-key fields, migration backfills, and reapply-safe migrations `0174`–`0175` after current master migrations. - Adds company-scoped folder CRUD, cycle/depth/namespace validation, reserved My/Projects/Bundled lifecycle, item moves, subtree filtering, and folder paths on skill results. - Preserves project-import placement: first import files into the project folder, while re-import keeps user-owned placement and stable skill IDs. - Adds the `/skills` folder tree rail, tags facet, breadcrumbs, subfolder browser, move/new-folder dialog, canonical detail location, inline tag editing, and folder-aware Studio creation. - Keeps bundled skills read-only even when their source metadata is incomplete by detecting the reserved Bundled folder and hiding selection/move actions. - Extends routine folder UI and OpenAPI coverage, and adds regression tests across migrations, services, routes, tree helpers, pages, and Studio creation. ## Verification - `pnpm exec vitest run packages/db/src/nested-skill-folders-migration.test.ts server/src/__tests__/folders-routes.test.ts server/src/__tests__/folders-service.test.ts server/src/__tests__/company-skills-service.test.ts server/src/__tests__/routines-service.test.ts ui/src/components/folders/FolderControls.test.tsx ui/src/components/folders/SkillFolderTree.test.tsx ui/src/components/folders/skill-folder-tree.test.ts ui/src/pages/CompanySkills.test.tsx ui/src/pages/Routines.test.tsx ui/src/pages/SkillStudio.test.tsx ui/src/lib/company-skill-routes.test.ts ui/src/lib/skill-create.test.ts` — 13 files, 192 tests passed. - `pnpm exec vitest run ui/src/pages/CompanySkills.test.tsx ui/src/components/folders/SkillFolderTree.test.tsx` — 2 files, 20 tests passed after preserving the existing PR's bundled-skill fixes. - `pnpm -r typecheck` — passed for all workspace packages. - `pnpm test:run` — passed in an isolated CI-like environment with inherited Paperclip runtime identity and static AWS credential variables removed. - `pnpm build` — production build passed for all workspace packages. - Greptile iteration 2 — 5/5 confidence with zero unresolved threads on commit `ff2d67aa71`. - Latest-head GitHub checks — all success, neutral, or skipped; PR is mergeable with a clean merge state. - `pnpm check:token-gates` — reports nine existing `#9627` comment false positives already present on `master`; this PR introduces no new token violation. ## Risks - **Migration/backfill:** `0174` creates the foundation and `0175` adds nested/reserved semantics. Both are ordered after current master migration `0173`, are covered by numbering/safety checks, and are designed to be reapply-safe. - **Reserved namespaces:** My, Projects, and Bundled roots are service-managed. Regression coverage prevents namespace squatting, cross-company folder use, bundled writes, cycles, and excessive depth. - **Behavioral change:** Project scans choose a project folder only on initial creation; existing skills deliberately retain their current folder during refresh. - **UI scope:** The folder rail applies to the Installed library; Catalog retains the discovery-oriented category sidebar. > 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.5` in Codex CLI, medium reasoning mode; runtime did not expose a context-window value. Used repository/file tools, terminal execution, Git/GitHub operations, test execution, and code editing. ## 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>
124 lines
6.3 KiB
TypeScript
124 lines
6.3 KiB
TypeScript
import { createHash, randomUUID } from "node:crypto";
|
|
import fs from "node:fs";
|
|
import { afterEach, describe, expect, it } from "vitest";
|
|
import postgres from "postgres";
|
|
import { applyPendingMigrations } from "./client.js";
|
|
import {
|
|
getEmbeddedPostgresTestSupport,
|
|
startEmbeddedPostgresTestDatabase,
|
|
} from "./test-embedded-postgres.js";
|
|
|
|
const MIGRATION_FILE = "0175_nested_skill_folders.sql";
|
|
const cleanups: Array<() => Promise<void>> = [];
|
|
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
|
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
|
|
|
async function migrationHash() {
|
|
const content = await fs.promises.readFile(new URL(`./migrations/${MIGRATION_FILE}`, import.meta.url), "utf8");
|
|
return createHash("sha256").update(content).digest("hex");
|
|
}
|
|
|
|
describeEmbeddedPostgres("nested skill folders migration", () => {
|
|
afterEach(async () => {
|
|
await Promise.all(cleanups.splice(0).map((cleanup) => cleanup()));
|
|
});
|
|
|
|
it("backfills existing folders, bundled skills, and project scan skills", async () => {
|
|
const database = await startEmbeddedPostgresTestDatabase("paperclip-nested-folders-migration-");
|
|
cleanups.push(database.cleanup);
|
|
const sql = postgres(database.connectionString, { max: 1 });
|
|
cleanups.push(async () => sql.end());
|
|
|
|
await sql`DELETE FROM "drizzle"."__drizzle_migrations" WHERE "hash" = ${await migrationHash()}`;
|
|
await sql`DROP INDEX IF EXISTS "folders_company_kind_parent_position_idx"`;
|
|
await sql`DROP INDEX IF EXISTS "folders_company_kind_system_key_uq"`;
|
|
await sql`DROP INDEX IF EXISTS "folders_company_kind_root_slug_uq"`;
|
|
await sql`DROP INDEX IF EXISTS "folders_company_kind_parent_slug_uq"`;
|
|
await sql`ALTER TABLE "folders" DROP CONSTRAINT IF EXISTS "folders_company_kind_parent_slug_uq"`;
|
|
await sql`ALTER TABLE "folders" DROP CONSTRAINT IF EXISTS "folders_parent_id_folders_id_fk"`;
|
|
await sql`ALTER TABLE "folders" DROP COLUMN IF EXISTS "system_key"`;
|
|
await sql`ALTER TABLE "folders" DROP COLUMN IF EXISTS "slug"`;
|
|
await sql`ALTER TABLE "folders" DROP COLUMN IF EXISTS "parent_id"`;
|
|
await sql`CREATE UNIQUE INDEX IF NOT EXISTS "folders_company_kind_name_uq" ON "folders" ("company_id", "kind", "name")`;
|
|
|
|
const companyId = randomUUID();
|
|
const projectId = randomUUID();
|
|
const existingFolderId = randomUUID();
|
|
const squattedBundledId = randomUUID();
|
|
const squattedProjectsId = randomUUID();
|
|
const bundledSkillId = randomUUID();
|
|
const projectSkillId = randomUUID();
|
|
const unfiledSkillId = randomUUID();
|
|
await sql`
|
|
INSERT INTO "companies" ("id", "name", "issue_prefix")
|
|
VALUES (${companyId}, 'Paperclip', 'PAP')
|
|
`;
|
|
await sql`
|
|
INSERT INTO "projects" ("id", "company_id", "name")
|
|
VALUES (${projectId}, ${companyId}, 'Agent Platform')
|
|
`;
|
|
await sql`
|
|
INSERT INTO "folders" ("id", "company_id", "kind", "name", "position")
|
|
VALUES
|
|
(${existingFolderId}, ${companyId}, 'skill', 'Team Notes', 0),
|
|
(${squattedBundledId}, ${companyId}, 'skill', 'Bundled', 1),
|
|
(${squattedProjectsId}, ${companyId}, 'skill', 'Projects', 2)
|
|
`;
|
|
await sql`
|
|
INSERT INTO "company_skills" ("id", "company_id", "key", "slug", "name", "markdown", "metadata")
|
|
VALUES
|
|
(${bundledSkillId}, ${companyId}, 'paperclipai/bundled/software-development/review', 'review', 'Review', '# Review', '{"sourceKind":"paperclip_bundled"}'::jsonb),
|
|
(${projectSkillId}, ${companyId}, 'company/project-skill', 'project-skill', 'Project Skill', '# Project', ${sql.json({ sourceKind: "project_scan", projectId, projectName: "Agent Platform" })}),
|
|
(${unfiledSkillId}, ${companyId}, 'company/unfiled', 'unfiled', 'Unfiled', '# Unfiled', '{}'::jsonb)
|
|
`;
|
|
|
|
await applyPendingMigrations(database.connectionString);
|
|
|
|
const folderRows = await sql<{
|
|
id: string;
|
|
parent_id: string | null;
|
|
slug: string;
|
|
system_key: string | null;
|
|
}[]>`
|
|
SELECT "id", "parent_id", "slug", "system_key"
|
|
FROM "folders"
|
|
WHERE "company_id" = ${companyId}
|
|
ORDER BY "slug"
|
|
`;
|
|
expect(folderRows).toEqual(expect.arrayContaining([
|
|
expect.objectContaining({ id: existingFolderId, slug: "team-notes", parent_id: null }),
|
|
expect.objectContaining({ id: squattedBundledId, slug: `bundled-${squattedBundledId.replace(/-/g, "").slice(0, 8)}`, system_key: null }),
|
|
expect.objectContaining({ id: squattedProjectsId, slug: `projects-${squattedProjectsId.replace(/-/g, "").slice(0, 8)}`, system_key: null }),
|
|
expect.objectContaining({ slug: "bundled", system_key: "bundled", parent_id: null }),
|
|
expect.objectContaining({ slug: "software-development", system_key: "bundled:software-development" }),
|
|
expect.objectContaining({ slug: "projects", system_key: "projects", parent_id: null }),
|
|
expect.objectContaining({ slug: "agent-platform", system_key: `project:${projectId}` }),
|
|
]));
|
|
|
|
const skills = await sql<{ id: string; folder_id: string | null; folder_slug: string | null }[]>`
|
|
SELECT skill."id", skill."folder_id", folder."slug" AS folder_slug
|
|
FROM "company_skills" AS skill
|
|
LEFT JOIN "folders" AS folder ON folder."id" = skill."folder_id"
|
|
WHERE skill."company_id" = ${companyId}
|
|
ORDER BY skill."id"
|
|
`;
|
|
expect(skills.find((skill) => skill.id === bundledSkillId)).toMatchObject({ folder_slug: "software-development" });
|
|
expect(skills.find((skill) => skill.id === projectSkillId)).toMatchObject({ folder_slug: "agent-platform" });
|
|
expect(skills.find((skill) => skill.id === unfiledSkillId)).toMatchObject({ folder_id: null, folder_slug: null });
|
|
|
|
const indexes = await sql<{ indexname: string; indexdef: string }[]>`
|
|
SELECT "indexname", "indexdef"
|
|
FROM "pg_indexes"
|
|
WHERE "tablename" = 'folders'
|
|
AND "indexname" IN ('folders_company_kind_root_slug_uq', 'folders_company_kind_parent_slug_uq')
|
|
ORDER BY "indexname"
|
|
`;
|
|
expect(indexes).toHaveLength(2);
|
|
expect(indexes.map((index) => index.indexdef).join("\n")).not.toContain("NULLS NOT DISTINCT");
|
|
expect(indexes.find((index) => index.indexname === "folders_company_kind_root_slug_uq")?.indexdef)
|
|
.toContain("WHERE (parent_id IS NULL)");
|
|
expect(indexes.find((index) => index.indexname === "folders_company_kind_parent_slug_uq")?.indexdef)
|
|
.toContain("WHERE (parent_id IS NOT NULL)");
|
|
}, 30_000);
|
|
});
|