mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 18:31:34 +01:00
Fix Clips social review findings
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
07f0df37c6
commit
bf6d6ed6e3
4 files changed
+306
-26
No files matched your search
@@ -41,6 +41,112 @@ vi.mock("../services/index.js", () => ({
|
||||
logActivity: mockLogActivity,
|
||||
}));
|
||||
|
||||
|
||||
const HASH = "sha256:" + "0".repeat(64);
|
||||
|
||||
function importableManifest(slug = "support-triage") {
|
||||
return {
|
||||
schema: "paperclip.clip/v1",
|
||||
clip: {
|
||||
id: "clip_1",
|
||||
slug,
|
||||
type: "bundle",
|
||||
revisionId: "cliprev_1",
|
||||
revisionNumber: 1,
|
||||
manifestVersion: 1,
|
||||
title: "Support Triage",
|
||||
summary: "Routes support tickets.",
|
||||
visibility: "public",
|
||||
creator: null,
|
||||
},
|
||||
publication: {
|
||||
source: {
|
||||
kind: "paperclip_company_object",
|
||||
objectType: "bundle",
|
||||
exportedAt: "2026-05-16T00:00:00.000Z",
|
||||
},
|
||||
compatibility: {
|
||||
paperclip: null,
|
||||
manifest: "agentcompanies/v1-draft",
|
||||
},
|
||||
},
|
||||
artifact: {
|
||||
format: "agentcompanies",
|
||||
version: "agentcompanies/v1-draft",
|
||||
checksum: HASH,
|
||||
entrypoint: "COMPANY.md",
|
||||
paperclipExtension: ".paperclip.yaml",
|
||||
payload: {
|
||||
manifest: { schemaVersion: 1 },
|
||||
files: { "COMPANY.md": "# Support Triage" },
|
||||
},
|
||||
},
|
||||
dependencies: {
|
||||
adapters: [],
|
||||
plugins: [],
|
||||
skills: [],
|
||||
secrets: [],
|
||||
permissions: [],
|
||||
runtime: {
|
||||
localShell: false,
|
||||
browser: false,
|
||||
filesystem: "none",
|
||||
webhooks: false,
|
||||
recurringRoutines: false,
|
||||
},
|
||||
workspaces: [],
|
||||
budgetHints: { monthlyCents: 0, sourceRefs: [] },
|
||||
},
|
||||
security: {
|
||||
redactionReport: {
|
||||
schema: "paperclip.clip.redaction/v1",
|
||||
generatedAt: "2026-05-16T00:00:00.000Z",
|
||||
entries: [],
|
||||
summary: { allowed: 0, redacted: 0, summarized: 0, omitted: 0 },
|
||||
},
|
||||
dangerousCapabilities: [],
|
||||
routinePolicy: {
|
||||
importedTriggersEnabledByDefault: false,
|
||||
webhookSecretsRegenerated: true,
|
||||
},
|
||||
reviewState: "unreviewed",
|
||||
},
|
||||
verification: {
|
||||
expectedFirstRun: null,
|
||||
sampleOutputs: [],
|
||||
validationStatus: "not_run",
|
||||
},
|
||||
social: {
|
||||
sourceUrl: null,
|
||||
revisionUrl: null,
|
||||
},
|
||||
provenance: {
|
||||
publishedByProfileId: null,
|
||||
revisionPublishedAt: "2026-05-16T00:00:00.000Z",
|
||||
previousRevisionId: null,
|
||||
},
|
||||
checksums: {
|
||||
artifact: HASH,
|
||||
redactionReport: HASH,
|
||||
manifest: HASH,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function publicClipFixture(input: { clipId: string; slug?: string }) {
|
||||
const slug = input.slug ?? "support-triage";
|
||||
return {
|
||||
id: input.clipId,
|
||||
slug,
|
||||
currentRevision: {
|
||||
revisionNumber: 1,
|
||||
manifestChecksum: HASH,
|
||||
artifactChecksum: HASH,
|
||||
manifestPayload: importableManifest(slug),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function createApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
@@ -72,4 +178,50 @@ describe("clip routes", () => {
|
||||
expect(mockCompanyService.getById).not.toHaveBeenCalled();
|
||||
expect(mockCompanyPortabilityService.previewExport).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("uses the clip source company when building import previews", async () => {
|
||||
const destinationCompanyId = "11111111-1111-4111-8111-111111111111";
|
||||
const sourceCompanyId = "22222222-2222-4222-8222-222222222222";
|
||||
const clipId = "33333333-3333-4333-8333-333333333333";
|
||||
mockClipService.getPublicDetail.mockResolvedValue(publicClipFixture({ clipId }));
|
||||
mockClipService.getClipById.mockResolvedValue({ id: clipId, sourceCompanyId });
|
||||
mockCompanyPortabilityService.previewImport.mockResolvedValue({ warnings: [] });
|
||||
|
||||
const res = await request(createApp())
|
||||
.post("/api/companies/" + destinationCompanyId + "/clips/import-preview")
|
||||
.send({ url: "https://paperclip.ing/clips/support-triage" });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockCompanyPortabilityService.previewImport).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ target: { mode: "existing_company", companyId: destinationCompanyId } }),
|
||||
expect.objectContaining({ mode: "agent_safe", sourceCompanyId }),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses the clip source company when applying imports", async () => {
|
||||
const destinationCompanyId = "11111111-1111-4111-8111-111111111111";
|
||||
const sourceCompanyId = "22222222-2222-4222-8222-222222222222";
|
||||
const clipId = "33333333-3333-4333-8333-333333333333";
|
||||
mockClipService.getPublicDetail.mockResolvedValue(publicClipFixture({ clipId }));
|
||||
mockClipService.getClipById.mockResolvedValue({ id: clipId, sourceCompanyId });
|
||||
mockCompanyPortabilityService.previewImport.mockResolvedValue({ warnings: [] });
|
||||
mockCompanyPortabilityService.importBundle.mockResolvedValue({
|
||||
agents: [],
|
||||
projects: [],
|
||||
issues: [],
|
||||
skills: [],
|
||||
warnings: [],
|
||||
});
|
||||
|
||||
const res = await request(createApp())
|
||||
.post("/api/companies/" + destinationCompanyId + "/clips/import")
|
||||
.send({ url: "https://paperclip.ing/clips/support-triage" });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockCompanyPortabilityService.importBundle).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ target: { mode: "existing_company", companyId: destinationCompanyId } }),
|
||||
"board",
|
||||
expect.objectContaining({ mode: "agent_safe", sourceCompanyId }),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -1,5 +1,5 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { asc, eq } from "drizzle-orm";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import {
|
||||
clipComments,
|
||||
@@ -183,4 +183,93 @@ describeEmbeddedPostgres("clipService", () => {
|
||||
expect(reports).toHaveLength(1);
|
||||
expect(reports[0]?.reason).toBe("unsafe_automation");
|
||||
});
|
||||
|
||||
it("serializes concurrent revision numbering for the same clip", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const { clip } = await publishFixture(companyId);
|
||||
|
||||
const [firstUpdate, secondUpdate] = await Promise.all([
|
||||
svc.createRevision(clip.id, {
|
||||
manifestChecksum: "sha256:manifest-concurrent-a",
|
||||
artifactChecksum: "sha256:artifact-concurrent-a",
|
||||
manifestPayload: { schema: "paperclip.clip/v1", revision: "a" },
|
||||
changeSummary: "Concurrent update A.",
|
||||
}),
|
||||
svc.createRevision(clip.id, {
|
||||
manifestChecksum: "sha256:manifest-concurrent-b",
|
||||
artifactChecksum: "sha256:artifact-concurrent-b",
|
||||
manifestPayload: { schema: "paperclip.clip/v1", revision: "b" },
|
||||
changeSummary: "Concurrent update B.",
|
||||
}),
|
||||
]);
|
||||
|
||||
expect([firstUpdate.revision.revisionNumber, secondUpdate.revision.revisionNumber].sort()).toEqual([2, 3]);
|
||||
const revisions = await db
|
||||
.select({ revisionNumber: clipRevisions.revisionNumber })
|
||||
.from(clipRevisions)
|
||||
.where(eq(clipRevisions.clipId, clip.id))
|
||||
.orderBy(asc(clipRevisions.revisionNumber));
|
||||
expect(revisions.map((revision) => revision.revisionNumber)).toEqual([1, 2, 3]);
|
||||
});
|
||||
|
||||
it("queries creator profile clips by creator before applying limits", async () => {
|
||||
const targetCompanyId = await seedCompany();
|
||||
const otherCompanyId = await seedCompany();
|
||||
const [targetProfile] = await db.insert(clipCreatorProfiles).values({
|
||||
companyId: targetCompanyId,
|
||||
handle: "target-" + targetCompanyId.slice(0, 8),
|
||||
displayName: "Target Creator",
|
||||
}).returning();
|
||||
const [otherProfile] = await db.insert(clipCreatorProfiles).values({
|
||||
companyId: otherCompanyId,
|
||||
handle: "other-" + otherCompanyId.slice(0, 8),
|
||||
displayName: "Other Creator",
|
||||
}).returning();
|
||||
if (!targetProfile || !otherProfile) {
|
||||
throw new Error("Expected creator profiles to be inserted");
|
||||
}
|
||||
|
||||
await db.insert(clips).values([
|
||||
{
|
||||
sourceCompanyId: targetCompanyId,
|
||||
creatorProfileId: targetProfile.id,
|
||||
slug: "target-public-" + targetCompanyId.slice(0, 8),
|
||||
type: "agent",
|
||||
title: "Target Public Clip",
|
||||
summary: "A public target clip.",
|
||||
visibility: "public",
|
||||
status: "published",
|
||||
updatedAt: new Date("2026-01-01T00:00:00.000Z"),
|
||||
},
|
||||
{
|
||||
sourceCompanyId: targetCompanyId,
|
||||
creatorProfileId: targetProfile.id,
|
||||
slug: "target-unlisted-" + targetCompanyId.slice(0, 8),
|
||||
type: "agent",
|
||||
title: "Target Unlisted Clip",
|
||||
summary: "An unlisted target clip.",
|
||||
visibility: "unlisted",
|
||||
status: "published",
|
||||
updatedAt: new Date("2026-01-02T00:00:00.000Z"),
|
||||
},
|
||||
...Array.from({ length: 100 }, (_, index) => ({
|
||||
sourceCompanyId: otherCompanyId,
|
||||
creatorProfileId: otherProfile.id,
|
||||
slug: "other-public-" + otherCompanyId.slice(0, 8) + "-" + index,
|
||||
type: "agent",
|
||||
title: "Other Public Clip " + index,
|
||||
summary: "A newer public clip from another creator.",
|
||||
visibility: "public",
|
||||
status: "published",
|
||||
updatedAt: new Date(Date.UTC(2026, 1, 1, 0, index)),
|
||||
})),
|
||||
]);
|
||||
|
||||
const profile = await svc.getCreatorPublicProfile(targetProfile.handle);
|
||||
|
||||
expect(profile?.clips.map((clip) => clip.slug).sort()).toEqual([
|
||||
"target-public-" + targetCompanyId.slice(0, 8),
|
||||
"target-unlisted-" + targetCompanyId.slice(0, 8),
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -31,7 +31,9 @@ import type { StorageService } from "../storage/types.js";
|
||||
|
||||
const CLIP_RATE_LIMIT_WINDOW_MS = 60_000;
|
||||
const CLIP_RATE_LIMIT_MAX_REQUESTS = 60;
|
||||
const CLIP_RATE_LIMIT_EVICTION_INTERVAL_MS = CLIP_RATE_LIMIT_WINDOW_MS;
|
||||
const clipRateLimitHits = new Map<string, number[]>();
|
||||
let clipRateLimitLastEvictionAt = 0;
|
||||
|
||||
function actorForClip(req: Request) {
|
||||
if (req.actor.type === "agent") {
|
||||
@@ -73,9 +75,23 @@ function getClipCompanyIdParam(req: Request) {
|
||||
return companyId;
|
||||
}
|
||||
|
||||
function evictExpiredClipRateLimitHits(cutoff: number, now: number) {
|
||||
if (now - clipRateLimitLastEvictionAt < CLIP_RATE_LIMIT_EVICTION_INTERVAL_MS) return;
|
||||
clipRateLimitLastEvictionAt = now;
|
||||
for (const [key, hits] of clipRateLimitHits) {
|
||||
const recent = hits.filter((hit) => hit > cutoff);
|
||||
if (recent.length > 0) {
|
||||
clipRateLimitHits.set(key, recent);
|
||||
} else {
|
||||
clipRateLimitHits.delete(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function consumeClipRateLimit(req: Request, res: { setHeader(name: string, value: string): void; status(code: number): { json(value: unknown): void } }, action: string) {
|
||||
const now = Date.now();
|
||||
const cutoff = now - CLIP_RATE_LIMIT_WINDOW_MS;
|
||||
evictExpiredClipRateLimitHits(cutoff, now);
|
||||
const actor = actorForClip(req);
|
||||
const ip = req.ip || req.socket.remoteAddress || "unknown-ip";
|
||||
const key = `${action}:${actor.actorType}:${actor.actorId}:${ip}`;
|
||||
@@ -188,6 +204,10 @@ export function clipRoutes(db: Db, storage?: StorageService) {
|
||||
if (!clip?.currentRevision) {
|
||||
throw unprocessable("Clip was not found or is not importable.");
|
||||
}
|
||||
const sourceClip = await svc.getClipById(clip.id);
|
||||
if (!sourceClip) {
|
||||
throw unprocessable("Clip source metadata was not found.");
|
||||
}
|
||||
const manifestPayload = clip.currentRevision.manifestPayload;
|
||||
const parsed = clipManifestSchema.safeParse(manifestPayload);
|
||||
if (!parsed.success) {
|
||||
@@ -204,13 +224,14 @@ export function clipRoutes(db: Db, storage?: StorageService) {
|
||||
collisionStrategy: input.collisionStrategy ?? "rename",
|
||||
}, {
|
||||
mode: "agent_safe",
|
||||
sourceCompanyId: companyId,
|
||||
sourceCompanyId: sourceClip.sourceCompanyId,
|
||||
});
|
||||
return {
|
||||
clip,
|
||||
parsedManifest: parsed.data,
|
||||
files,
|
||||
preview,
|
||||
sourceCompanyId: sourceClip.sourceCompanyId,
|
||||
source: {
|
||||
url: input.url,
|
||||
revisionNumber: clip.currentRevision.revisionNumber,
|
||||
@@ -534,7 +555,7 @@ export function clipRoutes(db: Db, storage?: StorageService) {
|
||||
collisionStrategy: req.body.collisionStrategy ?? "rename",
|
||||
}, req.actor.type === "board" ? req.actor.userId : null, {
|
||||
mode: "agent_safe",
|
||||
sourceCompanyId: companyId,
|
||||
sourceCompanyId: result.sourceCompanyId,
|
||||
});
|
||||
await svc.recordImportTelemetry(result.clip.slug, {
|
||||
revisionNumber: result.source.revisionNumber,
|
||||
|
||||
@@ -44,6 +44,8 @@ type Actor = {
|
||||
userId?: string | null;
|
||||
};
|
||||
|
||||
type ClipRevisionClient = Pick<Db, "insert" | "select">;
|
||||
|
||||
function first<T>(rows: T[]): T | null {
|
||||
return rows[0] ?? null;
|
||||
}
|
||||
@@ -172,14 +174,22 @@ export function clipService(db: Db) {
|
||||
q?: string | null;
|
||||
type?: string | null;
|
||||
tag?: string | null;
|
||||
creatorProfileId?: string | null;
|
||||
includeUnlisted?: boolean;
|
||||
limit?: number;
|
||||
offset?: number;
|
||||
} = {}) {
|
||||
const conditions = [
|
||||
eq(clips.visibility, "public"),
|
||||
input.includeUnlisted
|
||||
? inArray(clips.visibility, PUBLIC_DETAIL_VISIBILITIES)
|
||||
: eq(clips.visibility, "public"),
|
||||
eq(clips.status, PUBLIC_STATUS),
|
||||
inArray(clips.moderationState, PUBLIC_BROWSE_MODERATION_STATES),
|
||||
inArray(
|
||||
clips.moderationState,
|
||||
input.includeUnlisted ? PUBLIC_DETAIL_MODERATION_STATES : PUBLIC_BROWSE_MODERATION_STATES,
|
||||
),
|
||||
];
|
||||
if (input.creatorProfileId) conditions.push(eq(clips.creatorProfileId, input.creatorProfileId));
|
||||
if (input.type) conditions.push(eq(clips.type, input.type));
|
||||
if (input.q) {
|
||||
const q = `%${input.q.trim()}%`;
|
||||
@@ -266,9 +276,9 @@ export function clipService(db: Db) {
|
||||
return createCreatorProfile(companyId, input.creatorProfile);
|
||||
}
|
||||
|
||||
async function insertRevision(clipId: string, input: CreateClipRevision, requestedRevisionNumber?: number) {
|
||||
const revisionNumber = requestedRevisionNumber ?? await nextRevisionNumber(clipId);
|
||||
const [revision] = await db
|
||||
async function insertRevision(client: ClipRevisionClient, clipId: string, input: CreateClipRevision, requestedRevisionNumber?: number) {
|
||||
const revisionNumber = requestedRevisionNumber ?? await nextRevisionNumber(client, clipId);
|
||||
const [revision] = await client
|
||||
.insert(clipRevisions)
|
||||
.values({
|
||||
clipId,
|
||||
@@ -294,7 +304,7 @@ export function clipService(db: Db) {
|
||||
.returning();
|
||||
|
||||
if (input.dependencies?.length) {
|
||||
await db.insert(clipDependencies).values(
|
||||
await client.insert(clipDependencies).values(
|
||||
input.dependencies.map((dependency) => ({
|
||||
clipId,
|
||||
revisionId: revision.id,
|
||||
@@ -309,8 +319,8 @@ export function clipService(db: Db) {
|
||||
return revision;
|
||||
}
|
||||
|
||||
async function nextRevisionNumber(clipId: string) {
|
||||
const [row] = await db
|
||||
async function nextRevisionNumber(client: Pick<Db, "select">, clipId: string) {
|
||||
const [row] = await client
|
||||
.select({ maxRevision: sql<number>`coalesce(max(${clipRevisions.revisionNumber}), 0)` })
|
||||
.from(clipRevisions)
|
||||
.where(eq(clipRevisions.clipId, clipId));
|
||||
@@ -343,7 +353,7 @@ export function clipService(db: Db) {
|
||||
sourceObjectId: input.sourceObjectId ?? null,
|
||||
})
|
||||
.returning();
|
||||
const revision = await insertRevision(clip.id, input.revision, 1);
|
||||
const revision = await insertRevision(db, clip.id, input.revision, 1);
|
||||
const visibleRevisionId = clip.status === "published" ? revision.id : null;
|
||||
const [updated] = await db
|
||||
.update(clips)
|
||||
@@ -359,18 +369,22 @@ export function clipService(db: Db) {
|
||||
}
|
||||
|
||||
async function createRevision(clipId: string, input: CreateClipRevision) {
|
||||
const clip = await getClipById(clipId);
|
||||
if (!clip) throw notFound("Clip not found");
|
||||
const revision = await insertRevision(clip.id, input);
|
||||
const [updated] = await db
|
||||
.update(clips)
|
||||
.set({
|
||||
currentRevisionId: revision.id,
|
||||
latestApprovedRevisionId: clip.status === "published" && clip.moderationState !== "blocked" ? revision.id : clip.latestApprovedRevisionId,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(clips.id, clip.id))
|
||||
.returning();
|
||||
const { clip: updated, revision } = await db.transaction(async (tx) => {
|
||||
await tx.execute(sql`select ${clips.id} from ${clips} where ${clips.id} = ${clipId} for update`);
|
||||
const clip = first(await tx.select().from(clips).where(eq(clips.id, clipId)).limit(1));
|
||||
if (!clip) throw notFound("Clip not found");
|
||||
const revision = await insertRevision(tx, clip.id, input);
|
||||
const [updated] = await tx
|
||||
.update(clips)
|
||||
.set({
|
||||
currentRevisionId: revision.id,
|
||||
latestApprovedRevisionId: clip.status === "published" && clip.moderationState !== "blocked" ? revision.id : clip.latestApprovedRevisionId,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(clips.id, clip.id))
|
||||
.returning();
|
||||
return { clip: updated, revision };
|
||||
});
|
||||
await rebuildRankingSnapshot(updated);
|
||||
return { clip: updated, revision };
|
||||
}
|
||||
@@ -662,7 +676,11 @@ export function clipService(db: Db) {
|
||||
.limit(1),
|
||||
);
|
||||
if (!profile) return null;
|
||||
const publicClips = await listPublic({ limit: 100 });
|
||||
const publicClips = await listPublic({
|
||||
creatorProfileId: profile.id,
|
||||
includeUnlisted: true,
|
||||
limit: 100,
|
||||
});
|
||||
return {
|
||||
profile: {
|
||||
id: profile.id,
|
||||
@@ -674,7 +692,7 @@ export function clipService(db: Db) {
|
||||
verificationState: profile.verificationState,
|
||||
reputationSummary: profile.reputationSummary,
|
||||
},
|
||||
clips: publicClips.filter((clip) => clip.creatorProfileId === profile.id),
|
||||
clips: publicClips,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user