mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-15 18:30:39 +09:00
Restore feedback trace export fixes
This commit is contained in:
parent
ed95fc1dda
commit
00898e8194
9 changed files with 357 additions and 24 deletions
|
|
@ -1069,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"));
|
||||
|
|
@ -1106,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",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue