mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-14 01:50:39 +09:00
[codex] Harden create-agent skill governance (#4422)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Hiring agents is a governance-sensitive workflow because it grants roles, adapter config, skills, and execution capability > - The create-agent skill needs explicit templates and review guidance so hires are auditable and not over-permissioned > - Skill sync also needs to recognize bundled Paperclip skills consistently for Codex local agents > - This pull request expands create-agent role templates, adds a security-engineer template, and documents capability/secret-handling review requirements > - The benefit is safer, more repeatable agent creation with clearer approval payloads and less permission sprawl ## What Changed - Expanded `paperclip-create-agent` guidance for template selection, adjacent-template drafting, and role-specific review bars. - Added a Security Engineer agent template and collaboration/safety sections for Coder, QA, and UX Designer templates. - Hardened draft-review guidance around desired skills, external-system access, secrets, and confidential advisory handling. - Updated LLM agent-configuration guidance to point hiring workflows at the create-agent skill. - Added tests for bundled skill sync, create-agent skill injection, hire approval payloads, and LLM route guidance. ## Verification - `pnpm exec vitest run server/src/__tests__/agent-skills-routes.test.ts server/src/__tests__/codex-local-skill-injection.test.ts server/src/__tests__/codex-local-skill-sync.test.ts server/src/__tests__/llms-routes.test.ts server/src/__tests__/paperclip-skill-utils.test.ts --config server/vitest.config.ts` passed: 5 files, 23 tests. - `git diff --check public-gh/master..pap-2228-create-agent-governance -- . ':(exclude)ui/storybook-static'` passed. - Confirmed this PR does not include `pnpm-lock.yaml`. ## Risks - Low-to-medium risk: this primarily changes skills/docs and tests, but it affects future hiring guidance and approval expectations. - Reviewers should check whether the new Security Engineer template is too broad for default company installs. - No database migrations. > 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 coding agent based on GPT-5, with shell, git, Paperclip API, and GitHub CLI tool use in the local Paperclip workspace. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge Note: screenshot checklist item is not applicable; this PR changes skills, docs, and server tests. --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
77a72e28c2
commit
641eb44949
15 changed files with 702 additions and 62 deletions
|
|
@ -511,6 +511,53 @@ describe("agent skill routes", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("preserves hire source issues, icons, desired skills, and approval payload details", async () => {
|
||||
const db = createDb(true);
|
||||
const sourceIssueId = "22222222-2222-4222-8222-222222222222";
|
||||
|
||||
const res = await request(await createApp(db))
|
||||
.post("/api/companies/company-1/agent-hires")
|
||||
.send({
|
||||
name: "Security Engineer",
|
||||
role: "engineer",
|
||||
icon: "crown",
|
||||
adapterType: "claude_local",
|
||||
desiredSkills: ["paperclip"],
|
||||
adapterConfig: {},
|
||||
sourceIssueId,
|
||||
});
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(201);
|
||||
expect(mockAgentService.create).toHaveBeenCalledWith(
|
||||
"company-1",
|
||||
expect.objectContaining({
|
||||
icon: "crown",
|
||||
adapterConfig: expect.objectContaining({
|
||||
paperclipSkillSync: expect.objectContaining({
|
||||
desiredSkills: ["paperclipai/paperclip/paperclip"],
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expect(mockApprovalService.create).toHaveBeenCalledWith(
|
||||
"company-1",
|
||||
expect.objectContaining({
|
||||
payload: expect.objectContaining({
|
||||
icon: "crown",
|
||||
desiredSkills: ["paperclipai/paperclip/paperclip"],
|
||||
requestedConfigurationSnapshot: expect.objectContaining({
|
||||
desiredSkills: ["paperclipai/paperclip/paperclip"],
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expect(mockIssueApprovalService.linkManyForApproval).toHaveBeenCalledWith(
|
||||
"approval-1",
|
||||
[sourceIssueId],
|
||||
{ agentId: null, userId: "local-board" },
|
||||
);
|
||||
});
|
||||
|
||||
it("uses managed AGENTS config in hire approval payloads", async () => {
|
||||
const res = await request(await createApp(createDb(true)))
|
||||
.post("/api/companies/company-1/agent-hires")
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ async function createCustomSkill(root: string, skillName: string) {
|
|||
|
||||
describe("codex local adapter skill injection", () => {
|
||||
const paperclipKey = "paperclipai/paperclip/paperclip";
|
||||
const createAgentKey = "paperclipai/paperclip/paperclip-create-agent";
|
||||
const cleanupDirs = new Set<string>();
|
||||
|
||||
afterEach(async () => {
|
||||
|
|
@ -48,6 +49,7 @@ describe("codex local adapter skill injection", () => {
|
|||
cleanupDirs.add(skillsHome);
|
||||
|
||||
await createPaperclipRepoSkill(currentRepo, "paperclip");
|
||||
await createPaperclipRepoSkill(currentRepo, "paperclip-create-agent");
|
||||
await createPaperclipRepoSkill(oldRepo, "paperclip");
|
||||
await fs.symlink(path.join(oldRepo, "skills", "paperclip"), path.join(skillsHome, "paperclip"));
|
||||
|
||||
|
|
@ -58,23 +60,39 @@ describe("codex local adapter skill injection", () => {
|
|||
},
|
||||
{
|
||||
skillsHome,
|
||||
skillsEntries: [{
|
||||
key: paperclipKey,
|
||||
runtimeName: "paperclip",
|
||||
source: path.join(currentRepo, "skills", "paperclip"),
|
||||
}],
|
||||
skillsEntries: [
|
||||
{
|
||||
key: paperclipKey,
|
||||
runtimeName: "paperclip",
|
||||
source: path.join(currentRepo, "skills", "paperclip"),
|
||||
},
|
||||
{
|
||||
key: createAgentKey,
|
||||
runtimeName: "paperclip-create-agent",
|
||||
source: path.join(currentRepo, "skills", "paperclip-create-agent"),
|
||||
},
|
||||
],
|
||||
},
|
||||
);
|
||||
|
||||
expect(await fs.realpath(path.join(skillsHome, "paperclip"))).toBe(
|
||||
await fs.realpath(path.join(currentRepo, "skills", "paperclip")),
|
||||
);
|
||||
expect(await fs.realpath(path.join(skillsHome, "paperclip-create-agent"))).toBe(
|
||||
await fs.realpath(path.join(currentRepo, "skills", "paperclip-create-agent")),
|
||||
);
|
||||
expect(logs).toContainEqual(
|
||||
expect.objectContaining({
|
||||
stream: "stdout",
|
||||
chunk: expect.stringContaining('Repaired Codex skill "paperclip"'),
|
||||
}),
|
||||
);
|
||||
expect(logs).toContainEqual(
|
||||
expect.objectContaining({
|
||||
stream: "stdout",
|
||||
chunk: expect.stringContaining('Injected Codex skill "paperclip-create-agent"'),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves a custom Codex skill symlink outside Paperclip repo checkouts", async () => {
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ async function makeTempDir(prefix: string): Promise<string> {
|
|||
|
||||
describe("codex local skill sync", () => {
|
||||
const paperclipKey = "paperclipai/paperclip/paperclip";
|
||||
const createAgentKey = "paperclipai/paperclip/paperclip-create-agent";
|
||||
const cleanupDirs = new Set<string>();
|
||||
|
||||
afterEach(async () => {
|
||||
|
|
@ -41,8 +42,11 @@ describe("codex local skill sync", () => {
|
|||
const before = await listCodexSkills(ctx);
|
||||
expect(before.mode).toBe("ephemeral");
|
||||
expect(before.desiredSkills).toContain(paperclipKey);
|
||||
expect(before.desiredSkills).toContain(createAgentKey);
|
||||
expect(before.entries.find((entry) => entry.key === paperclipKey)?.required).toBe(true);
|
||||
expect(before.entries.find((entry) => entry.key === paperclipKey)?.state).toBe("configured");
|
||||
expect(before.entries.find((entry) => entry.key === createAgentKey)?.required).toBe(true);
|
||||
expect(before.entries.find((entry) => entry.key === createAgentKey)?.state).toBe("configured");
|
||||
expect(before.entries.find((entry) => entry.key === paperclipKey)?.detail).toContain("CODEX_HOME/skills/");
|
||||
});
|
||||
|
||||
|
|
@ -92,7 +96,9 @@ describe("codex local skill sync", () => {
|
|||
|
||||
const after = await syncCodexSkills(configuredCtx, []);
|
||||
expect(after.desiredSkills).toContain(paperclipKey);
|
||||
expect(after.desiredSkills).toContain(createAgentKey);
|
||||
expect(after.entries.find((entry) => entry.key === paperclipKey)?.state).toBe("configured");
|
||||
expect(after.entries.find((entry) => entry.key === createAgentKey)?.state).toBe("configured");
|
||||
});
|
||||
|
||||
it("normalizes legacy flat Paperclip skill refs before reporting configured state", async () => {
|
||||
|
|
|
|||
|
|
@ -123,6 +123,7 @@ describeEmbeddedPostgres("heartbeat dependency-aware queued run selection", () =
|
|||
await db.delete(issueTreeHolds);
|
||||
await db.delete(issues);
|
||||
await db.delete(heartbeatRunEvents);
|
||||
await db.delete(activityLog);
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(agentWakeupRequests);
|
||||
await db.delete(agentRuntimeState);
|
||||
|
|
|
|||
|
|
@ -66,6 +66,9 @@ describe("llm routes", () => {
|
|||
const res = await request(app).get("/api/llms/agent-configuration.txt");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.text).toContain("Use the paperclip-create-agent skill for end-to-end hiring");
|
||||
expect(res.text).toContain("desiredSkills");
|
||||
expect(res.text).toContain("sourceIssueId/sourceIssueIds");
|
||||
expect(res.text).toContain("Timer heartbeats are opt-in for new hires.");
|
||||
expect(res.text).toContain("Leave runtimeConfig.heartbeat.enabled false");
|
||||
});
|
||||
|
|
|
|||
|
|
@ -19,20 +19,28 @@ describe("paperclip skill utils", () => {
|
|||
cleanupDirs.clear();
|
||||
});
|
||||
|
||||
it("lists runtime skills from ./skills without pulling in .agents/skills", async () => {
|
||||
it("lists bundled runtime skills from ./skills without pulling in .agents/skills", async () => {
|
||||
const root = await makeTempDir("paperclip-skill-roots-");
|
||||
cleanupDirs.add(root);
|
||||
|
||||
const moduleDir = path.join(root, "a", "b", "c", "d", "e");
|
||||
await fs.mkdir(moduleDir, { recursive: true });
|
||||
await fs.mkdir(path.join(root, "skills", "paperclip"), { recursive: true });
|
||||
await fs.mkdir(path.join(root, "skills", "paperclip-create-agent"), { recursive: true });
|
||||
await fs.mkdir(path.join(root, ".agents", "skills", "release"), { recursive: true });
|
||||
|
||||
const entries = await listPaperclipSkillEntries(moduleDir);
|
||||
|
||||
expect(entries.map((entry) => entry.key)).toEqual(["paperclipai/paperclip/paperclip"]);
|
||||
expect(entries.map((entry) => entry.runtimeName)).toEqual(["paperclip"]);
|
||||
expect(entries.map((entry) => entry.key)).toEqual([
|
||||
"paperclipai/paperclip/paperclip",
|
||||
"paperclipai/paperclip/paperclip-create-agent",
|
||||
]);
|
||||
expect(entries.map((entry) => entry.runtimeName)).toEqual([
|
||||
"paperclip",
|
||||
"paperclip-create-agent",
|
||||
]);
|
||||
expect(entries[0]?.source).toBe(path.join(root, "skills", "paperclip"));
|
||||
expect(entries[1]?.source).toBe(path.join(root, "skills", "paperclip-create-agent"));
|
||||
});
|
||||
|
||||
it("removes stale maintainer-only symlinks from a shared skills home", async () => {
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ export function llmRoutes(db: Db) {
|
|||
"Notes:",
|
||||
"- Sensitive values are redacted in configuration read APIs.",
|
||||
"- New hires may be created in pending_approval state depending on company settings.",
|
||||
"- Use the paperclip-create-agent skill for end-to-end hiring: adapter reflection, config comparison, instruction source selection, icon choice, desiredSkills, sourceIssueId/sourceIssueIds, and approval follow-up.",
|
||||
"- Timer heartbeats are opt-in for new hires. Leave runtimeConfig.heartbeat.enabled false unless the role truly needs scheduled work or the user explicitly asked for it.",
|
||||
"",
|
||||
];
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue