mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-14 18:10:39 +09:00
Merge remote-tracking branch 'upstream/master' into feat/external-adapter-phase1
This commit is contained in:
commit
80b81459a7
10 changed files with 366 additions and 29 deletions
|
|
@ -187,7 +187,11 @@ describe("feedbackService.saveIssueVote", () => {
|
|||
const targetCommentId = randomUUID();
|
||||
const earlierCommentId = randomUUID();
|
||||
const laterCommentId = randomUUID();
|
||||
const runId = randomUUID();
|
||||
// Use a deterministic UUID whose hyphen-separated segments cannot be
|
||||
// mistaken for a phone number by the PII redactor's phone regex.
|
||||
// Random UUIDs occasionally produce digit pairs like "4880-8614" that
|
||||
// cross segment boundaries and match the phone pattern.
|
||||
const runId = "abcde123-face-beef-cafe-abcdef654321";
|
||||
const instructionsDir = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-feedback-instructions-"));
|
||||
tempDirs.push(instructionsDir);
|
||||
const instructionsPath = path.join(instructionsDir, "AGENTS.md");
|
||||
|
|
@ -1065,6 +1069,73 @@ describe("feedbackService.saveIssueVote", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("can flush a single shared trace immediately by trace id", async () => {
|
||||
const { companyId, issueId, commentId: firstCommentId } = await seedIssueWithAgentComment();
|
||||
const secondCommentId = randomUUID();
|
||||
const agentId = await db
|
||||
.select({ authorAgentId: issueComments.authorAgentId })
|
||||
.from(issueComments)
|
||||
.where(eq(issueComments.id, firstCommentId))
|
||||
.then((rows) => rows[0]?.authorAgentId ?? null);
|
||||
|
||||
await db.insert(issueComments).values({
|
||||
id: secondCommentId,
|
||||
companyId,
|
||||
issueId,
|
||||
authorAgentId: agentId,
|
||||
body: "Second AI generated update",
|
||||
});
|
||||
|
||||
const uploadTraceBundle = vi.fn().mockResolvedValue({
|
||||
objectKey: `feedback-traces/${companyId}/2026/04/01/test-trace.json`,
|
||||
});
|
||||
const flushingSvc = feedbackService(db, {
|
||||
shareClient: {
|
||||
uploadTraceBundle,
|
||||
},
|
||||
});
|
||||
|
||||
const first = await flushingSvc.saveIssueVote({
|
||||
issueId,
|
||||
targetType: "issue_comment",
|
||||
targetId: firstCommentId,
|
||||
vote: "up",
|
||||
authorUserId: "user-1",
|
||||
allowSharing: true,
|
||||
});
|
||||
await flushingSvc.saveIssueVote({
|
||||
issueId,
|
||||
targetType: "issue_comment",
|
||||
targetId: secondCommentId,
|
||||
vote: "up",
|
||||
authorUserId: "user-1",
|
||||
allowSharing: true,
|
||||
});
|
||||
|
||||
const flushResult = await flushingSvc.flushPendingFeedbackTraces({
|
||||
companyId,
|
||||
traceId: first.traceId ?? undefined,
|
||||
limit: 1,
|
||||
});
|
||||
|
||||
expect(flushResult).toMatchObject({
|
||||
attempted: 1,
|
||||
sent: 1,
|
||||
failed: 0,
|
||||
});
|
||||
expect(uploadTraceBundle).toHaveBeenCalledTimes(1);
|
||||
|
||||
const traces = await flushingSvc.listFeedbackTraces({
|
||||
companyId,
|
||||
issueId,
|
||||
includePayload: true,
|
||||
});
|
||||
const firstTrace = traces.find((trace) => trace.targetId === firstCommentId);
|
||||
const secondTrace = traces.find((trace) => trace.targetId === secondCommentId);
|
||||
expect(firstTrace?.status).toBe("sent");
|
||||
expect(secondTrace?.status).toBe("pending");
|
||||
});
|
||||
|
||||
it("marks pending shared traces as failed when remote export upload fails", async () => {
|
||||
const { companyId, issueId, commentId } = await seedIssueWithAgentComment();
|
||||
const uploadTraceBundle = vi.fn().mockRejectedValue(new Error("telemetry unavailable"));
|
||||
|
|
@ -1102,4 +1173,39 @@ describe("feedbackService.saveIssueVote", () => {
|
|||
expect(traces[0]?.exportedAt).toBeNull();
|
||||
expect(uploadTraceBundle).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("marks pending shared traces as failed when no feedback export backend is configured", async () => {
|
||||
const { companyId, issueId, commentId } = await seedIssueWithAgentComment();
|
||||
|
||||
const result = await svc.saveIssueVote({
|
||||
issueId,
|
||||
targetType: "issue_comment",
|
||||
targetId: commentId,
|
||||
vote: "up",
|
||||
authorUserId: "user-1",
|
||||
allowSharing: true,
|
||||
});
|
||||
|
||||
const flushResult = await svc.flushPendingFeedbackTraces({
|
||||
companyId,
|
||||
traceId: result.traceId ?? undefined,
|
||||
limit: 1,
|
||||
});
|
||||
|
||||
expect(flushResult).toMatchObject({
|
||||
attempted: 1,
|
||||
sent: 0,
|
||||
failed: 1,
|
||||
});
|
||||
|
||||
const traces = await svc.listFeedbackTraces({
|
||||
companyId,
|
||||
issueId,
|
||||
includePayload: true,
|
||||
});
|
||||
expect(traces[0]?.status).toBe("failed");
|
||||
expect(traces[0]?.attemptCount).toBe(1);
|
||||
expect(traces[0]?.failureReason).toBe("Feedback export backend is not configured");
|
||||
expect(traces[0]?.exportedAt).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
101
server/src/__tests__/feedback-share-client.test.ts
Normal file
101
server/src/__tests__/feedback-share-client.test.ts
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
import { gunzipSync } from "node:zlib";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { createFeedbackTraceShareClientFromConfig } from "../services/feedback-share-client.js";
|
||||
|
||||
describe("feedback trace share client", () => {
|
||||
beforeEach(() => {
|
||||
vi.stubGlobal("fetch", vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: vi.fn().mockResolvedValue({ objectKey: "feedback-traces/test.json" }),
|
||||
}));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("defaults to telemetry.paperclip.ing when no backend url is configured", async () => {
|
||||
const client = createFeedbackTraceShareClientFromConfig({
|
||||
feedbackExportBackendUrl: undefined,
|
||||
feedbackExportBackendToken: undefined,
|
||||
});
|
||||
|
||||
await client.uploadTraceBundle({
|
||||
traceId: "trace-1",
|
||||
exportId: "export-1",
|
||||
companyId: "company-1",
|
||||
issueId: "issue-1",
|
||||
issueIdentifier: "PAP-1",
|
||||
adapterType: "codex_local",
|
||||
captureStatus: "full",
|
||||
notes: [],
|
||||
envelope: {},
|
||||
surface: null,
|
||||
paperclipRun: null,
|
||||
rawAdapterTrace: null,
|
||||
normalizedAdapterTrace: null,
|
||||
privacy: null,
|
||||
integrity: {},
|
||||
files: [],
|
||||
});
|
||||
|
||||
expect(fetch).toHaveBeenCalledWith(
|
||||
"https://telemetry.paperclip.ing/feedback-traces",
|
||||
expect.objectContaining({
|
||||
method: "POST",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("wraps the feedback trace payload as gzip+base64 json before upload", async () => {
|
||||
const client = createFeedbackTraceShareClientFromConfig({
|
||||
feedbackExportBackendUrl: "https://telemetry.paperclip.ing",
|
||||
feedbackExportBackendToken: "test-token",
|
||||
});
|
||||
|
||||
await client.uploadTraceBundle({
|
||||
traceId: "trace-1",
|
||||
exportId: "export-1",
|
||||
companyId: "company-1",
|
||||
issueId: "issue-1",
|
||||
issueIdentifier: "PAP-1",
|
||||
adapterType: "codex_local",
|
||||
captureStatus: "full",
|
||||
notes: [],
|
||||
envelope: { hello: "world" },
|
||||
surface: null,
|
||||
paperclipRun: null,
|
||||
rawAdapterTrace: null,
|
||||
normalizedAdapterTrace: null,
|
||||
privacy: null,
|
||||
integrity: {},
|
||||
files: [],
|
||||
});
|
||||
|
||||
const call = vi.mocked(fetch).mock.calls[0];
|
||||
expect(call?.[0]).toBe("https://telemetry.paperclip.ing/feedback-traces");
|
||||
expect(call?.[1]).toMatchObject({
|
||||
method: "POST",
|
||||
headers: {
|
||||
"content-type": "application/json",
|
||||
authorization: "Bearer test-token",
|
||||
},
|
||||
});
|
||||
|
||||
const body = JSON.parse(String(call?.[1]?.body ?? "{}")) as {
|
||||
encoding?: string;
|
||||
payload?: string;
|
||||
};
|
||||
expect(body.encoding).toBe("gzip+base64+json");
|
||||
expect(typeof body.payload).toBe("string");
|
||||
|
||||
const decoded = gunzipSync(Buffer.from(body.payload ?? "", "base64")).toString("utf8");
|
||||
const parsed = JSON.parse(decoded) as {
|
||||
objectKey: string;
|
||||
bundle: { envelope: { hello: string } };
|
||||
};
|
||||
expect(parsed.objectKey).toContain("feedback-traces/company-1/");
|
||||
expect(parsed.objectKey.endsWith("/export-1.json")).toBe(true);
|
||||
expect(parsed.bundle.envelope).toEqual({ hello: "world" });
|
||||
});
|
||||
});
|
||||
|
|
@ -12,6 +12,18 @@ const mockFeedbackService = vi.hoisted(() => ({
|
|||
saveIssueVote: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockIssueService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
getByIdentifier: vi.fn(),
|
||||
update: vi.fn(),
|
||||
addComment: vi.fn(),
|
||||
findMentionedAgents: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockFeedbackExportService = vi.hoisted(() => ({
|
||||
flushPendingFeedbackTraces: vi.fn(async () => ({ attempted: 1, sent: 1, failed: 0 })),
|
||||
}));
|
||||
|
||||
vi.mock("../services/index.js", () => ({
|
||||
accessService: () => ({
|
||||
canUser: vi.fn(),
|
||||
|
|
@ -42,12 +54,7 @@ vi.mock("../services/index.js", () => ({
|
|||
listCompanyIds: vi.fn(async () => ["company-1"]),
|
||||
}),
|
||||
issueApprovalService: () => ({}),
|
||||
issueService: () => ({
|
||||
getById: vi.fn(),
|
||||
update: vi.fn(),
|
||||
addComment: vi.fn(),
|
||||
findMentionedAgents: vi.fn(),
|
||||
}),
|
||||
issueService: () => mockIssueService,
|
||||
logActivity: vi.fn(async () => undefined),
|
||||
projectService: () => ({}),
|
||||
routineService: () => ({
|
||||
|
|
@ -63,7 +70,7 @@ function createApp(actor: Record<string, unknown>) {
|
|||
(req as any).actor = actor;
|
||||
next();
|
||||
});
|
||||
app.use("/api", issueRoutes({} as any, {} as any));
|
||||
app.use("/api", issueRoutes({} as any, {} as any, { feedbackExportService: mockFeedbackExportService }));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
|
@ -73,6 +80,50 @@ describe("issue feedback trace routes", () => {
|
|||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("flushes a newly shared feedback trace immediately after saving the vote", async () => {
|
||||
const targetId = "11111111-1111-4111-8111-111111111111";
|
||||
mockIssueService.getById.mockResolvedValue({
|
||||
id: "issue-1",
|
||||
companyId: "company-1",
|
||||
identifier: "PAP-1",
|
||||
});
|
||||
mockFeedbackService.saveIssueVote.mockResolvedValue({
|
||||
vote: {
|
||||
targetType: "issue_comment",
|
||||
targetId,
|
||||
vote: "up",
|
||||
reason: null,
|
||||
},
|
||||
traceId: "trace-1",
|
||||
consentEnabledNow: false,
|
||||
persistedSharingPreference: null,
|
||||
sharingEnabled: true,
|
||||
});
|
||||
const app = createApp({
|
||||
type: "board",
|
||||
userId: "user-1",
|
||||
source: "session",
|
||||
isInstanceAdmin: true,
|
||||
companyIds: ["company-1"],
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/issues/issue-1/feedback-votes")
|
||||
.send({
|
||||
targetType: "issue_comment",
|
||||
targetId,
|
||||
vote: "up",
|
||||
allowSharing: true,
|
||||
});
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(mockFeedbackExportService.flushPendingFeedbackTraces).toHaveBeenCalledWith({
|
||||
companyId: "company-1",
|
||||
traceId: "trace-1",
|
||||
limit: 1,
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects non-board callers before fetching a feedback trace", async () => {
|
||||
const app = createApp({
|
||||
type: "agent",
|
||||
|
|
|
|||
|
|
@ -68,6 +68,7 @@ export async function createApp(
|
|||
feedbackExportService?: {
|
||||
flushPendingFeedbackTraces(input?: {
|
||||
companyId?: string;
|
||||
traceId?: string;
|
||||
limit?: number;
|
||||
now?: Date;
|
||||
}): Promise<unknown>;
|
||||
|
|
@ -153,7 +154,9 @@ export async function createApp(
|
|||
api.use(agentRoutes(db));
|
||||
api.use(assetRoutes(db, opts.storageService));
|
||||
api.use(projectRoutes(db));
|
||||
api.use(issueRoutes(db, opts.storageService));
|
||||
api.use(issueRoutes(db, opts.storageService, {
|
||||
feedbackExportService: opts.feedbackExportService,
|
||||
}));
|
||||
api.use(routineRoutes(db));
|
||||
api.use(executionWorkspaceRoutes(db));
|
||||
api.use(goalRoutes(db));
|
||||
|
|
|
|||
|
|
@ -525,7 +525,7 @@ export async function startServer(): Promise<StartedServer> {
|
|||
const uiMode = config.uiDevMiddleware ? "vite-dev" : config.serveUi ? "static" : "none";
|
||||
const storageService = createStorageServiceFromConfig(config);
|
||||
const feedback = feedbackService(db as any, {
|
||||
shareClient: createFeedbackTraceShareClientFromConfig(config) ?? undefined,
|
||||
shareClient: createFeedbackTraceShareClientFromConfig(config),
|
||||
});
|
||||
const app = await createApp(db as any, {
|
||||
uiMode,
|
||||
|
|
|
|||
|
|
@ -52,7 +52,20 @@ const updateIssueRouteSchema = updateIssueSchema.extend({
|
|||
interrupt: z.boolean().optional(),
|
||||
});
|
||||
|
||||
export function issueRoutes(db: Db, storage: StorageService) {
|
||||
export function issueRoutes(
|
||||
db: Db,
|
||||
storage: StorageService,
|
||||
opts?: {
|
||||
feedbackExportService?: {
|
||||
flushPendingFeedbackTraces(input?: {
|
||||
companyId?: string;
|
||||
traceId?: string;
|
||||
limit?: number;
|
||||
now?: Date;
|
||||
}): Promise<unknown>;
|
||||
};
|
||||
},
|
||||
) {
|
||||
const router = Router();
|
||||
const svc = issueService(db);
|
||||
const access = accessService(db);
|
||||
|
|
@ -67,6 +80,7 @@ export function issueRoutes(db: Db, storage: StorageService) {
|
|||
const workProductsSvc = workProductService(db);
|
||||
const documentsSvc = documentService(db);
|
||||
const routinesSvc = routineService(db);
|
||||
const feedbackExportService = opts?.feedbackExportService;
|
||||
const upload = multer({
|
||||
storage: multer.memoryStorage(),
|
||||
limits: { fileSize: MAX_ATTACHMENT_BYTES, files: 1 },
|
||||
|
|
@ -1867,6 +1881,18 @@ export function issueRoutes(db: Db, storage: StorageService) {
|
|||
);
|
||||
}
|
||||
|
||||
if (result.sharingEnabled && result.traceId && feedbackExportService) {
|
||||
try {
|
||||
await feedbackExportService.flushPendingFeedbackTraces({
|
||||
companyId: issue.companyId,
|
||||
traceId: result.traceId,
|
||||
limit: 1,
|
||||
});
|
||||
} catch (err) {
|
||||
logger.warn({ err, issueId: issue.id, traceId: result.traceId }, "failed to flush shared feedback trace immediately");
|
||||
}
|
||||
}
|
||||
|
||||
res.status(201).json(result.vote);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,9 @@
|
|||
import { gzipSync } from "node:zlib";
|
||||
import type { FeedbackTraceBundle } from "@paperclipai/shared";
|
||||
import type { Config } from "../config.js";
|
||||
|
||||
const DEFAULT_FEEDBACK_EXPORT_BACKEND_URL = "https://telemetry.paperclip.ing";
|
||||
|
||||
function buildFeedbackShareObjectKey(bundle: FeedbackTraceBundle, exportedAt: Date) {
|
||||
const year = String(exportedAt.getUTCFullYear());
|
||||
const month = String(exportedAt.getUTCMonth() + 1).padStart(2, "0");
|
||||
|
|
@ -14,10 +17,8 @@ export interface FeedbackTraceShareClient {
|
|||
|
||||
export function createFeedbackTraceShareClientFromConfig(
|
||||
config: Pick<Config, "feedbackExportBackendUrl" | "feedbackExportBackendToken">,
|
||||
): FeedbackTraceShareClient | null {
|
||||
const baseUrl = config.feedbackExportBackendUrl?.trim();
|
||||
if (!baseUrl) return null;
|
||||
|
||||
): FeedbackTraceShareClient {
|
||||
const baseUrl = config.feedbackExportBackendUrl?.trim() || DEFAULT_FEEDBACK_EXPORT_BACKEND_URL;
|
||||
const token = config.feedbackExportBackendToken?.trim();
|
||||
const endpoint = new URL("/feedback-traces", baseUrl).toString();
|
||||
|
||||
|
|
@ -25,6 +26,11 @@ export function createFeedbackTraceShareClientFromConfig(
|
|||
async uploadTraceBundle(bundle) {
|
||||
const exportedAt = new Date();
|
||||
const objectKey = buildFeedbackShareObjectKey(bundle, exportedAt);
|
||||
const requestBody = JSON.stringify({
|
||||
objectKey,
|
||||
exportedAt: exportedAt.toISOString(),
|
||||
bundle,
|
||||
});
|
||||
const response = await fetch(endpoint, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
|
|
@ -32,9 +38,8 @@ export function createFeedbackTraceShareClientFromConfig(
|
|||
...(token ? { authorization: `Bearer ${token}` } : {}),
|
||||
},
|
||||
body: JSON.stringify({
|
||||
objectKey,
|
||||
exportedAt: exportedAt.toISOString(),
|
||||
bundle,
|
||||
encoding: "gzip+base64+json",
|
||||
payload: gzipSync(requestBody).toString("base64"),
|
||||
}),
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -63,6 +63,7 @@ const MAX_SKILLS = 20;
|
|||
const MAX_INSTRUCTION_FILES = 20;
|
||||
const MAX_TRACE_FILE_CHARS = 10_000_000;
|
||||
const DEFAULT_INSTANCE_SETTINGS_SINGLETON_KEY = "default";
|
||||
const FEEDBACK_EXPORT_BACKEND_NOT_CONFIGURED = "Feedback export backend is not configured";
|
||||
|
||||
type FeedbackTraceRow = typeof feedbackExports.$inferSelect & {
|
||||
issueIdentifier: string | null;
|
||||
|
|
@ -1742,15 +1743,48 @@ export function feedbackService(db: Db, options: FeedbackServiceOptions = {}) {
|
|||
|
||||
flushPendingFeedbackTraces: async (input?: {
|
||||
companyId?: string;
|
||||
traceId?: string;
|
||||
limit?: number;
|
||||
now?: Date;
|
||||
}) => {
|
||||
const shareClient = options.shareClient;
|
||||
if (!shareClient) {
|
||||
const filters = [eq(feedbackExports.status, "pending")];
|
||||
if (input?.companyId) {
|
||||
filters.push(eq(feedbackExports.companyId, input.companyId));
|
||||
}
|
||||
if (input?.traceId) {
|
||||
filters.push(eq(feedbackExports.id, input.traceId));
|
||||
}
|
||||
|
||||
const rows = await db
|
||||
.select({
|
||||
id: feedbackExports.id,
|
||||
attemptCount: feedbackExports.attemptCount,
|
||||
})
|
||||
.from(feedbackExports)
|
||||
.where(and(...filters))
|
||||
.orderBy(asc(feedbackExports.createdAt), asc(feedbackExports.id))
|
||||
.limit(Math.max(1, Math.min(input?.limit ?? 25, 200)));
|
||||
|
||||
const attemptAt = input?.now ?? new Date();
|
||||
for (const row of rows) {
|
||||
await db
|
||||
.update(feedbackExports)
|
||||
.set({
|
||||
status: "failed",
|
||||
attemptCount: row.attemptCount + 1,
|
||||
lastAttemptedAt: attemptAt,
|
||||
failureReason: FEEDBACK_EXPORT_BACKEND_NOT_CONFIGURED,
|
||||
updatedAt: attemptAt,
|
||||
})
|
||||
.where(eq(feedbackExports.id, row.id));
|
||||
}
|
||||
|
||||
return {
|
||||
attempted: 0,
|
||||
attempted: rows.length,
|
||||
sent: 0,
|
||||
failed: 0,
|
||||
failed: rows.length,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -1761,6 +1795,9 @@ export function feedbackService(db: Db, options: FeedbackServiceOptions = {}) {
|
|||
if (input?.companyId) {
|
||||
filters.push(eq(feedbackExports.companyId, input.companyId));
|
||||
}
|
||||
if (input?.traceId) {
|
||||
filters.push(eq(feedbackExports.id, input.traceId));
|
||||
}
|
||||
|
||||
const rows = await db
|
||||
.select({
|
||||
|
|
@ -1983,7 +2020,7 @@ export function feedbackService(db: Db, options: FeedbackServiceOptions = {}) {
|
|||
})
|
||||
.where(eq(feedbackVotes.id, savedVote.id));
|
||||
|
||||
await tx
|
||||
const [savedTrace] = await tx
|
||||
.insert(feedbackExports)
|
||||
.values({
|
||||
companyId: issue.companyId,
|
||||
|
|
@ -2030,6 +2067,9 @@ export function feedbackService(db: Db, options: FeedbackServiceOptions = {}) {
|
|||
failureReason: null,
|
||||
updatedAt: now,
|
||||
},
|
||||
})
|
||||
.returning({
|
||||
id: feedbackExports.id,
|
||||
});
|
||||
|
||||
return {
|
||||
|
|
@ -2037,6 +2077,7 @@ export function feedbackService(db: Db, options: FeedbackServiceOptions = {}) {
|
|||
...savedVote,
|
||||
redactionSummary: artifacts.redactionSummary,
|
||||
},
|
||||
traceId: savedTrace?.id ?? null,
|
||||
consentEnabledNow,
|
||||
persistedSharingPreference,
|
||||
sharingEnabled: sharedWithLabs,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue