feat(artifacts): bridge Markdown work products into the document review surface (#11822)
This commit is contained in:
Michael Nguyen
GitHub
parent
733ffbf7c3
commit
0fa318b8da
|
|
@ -84,6 +84,19 @@ export {
|
|||
type ResponsibleUserSource,
|
||||
type OriginatingActor,
|
||||
} from "./issue-attribution.js";
|
||||
export {
|
||||
ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX,
|
||||
MARKDOWN_ATTACHMENT_CONTENT_TYPES,
|
||||
MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES,
|
||||
artifactReviewDocumentKey,
|
||||
getAttachmentArtifactWorkProductMetadata,
|
||||
getMarkdownWorkProductAttachmentMetadata,
|
||||
isArtifactReviewDocumentKey,
|
||||
isMarkdownArtifactWorkProduct,
|
||||
isMarkdownAttachmentContent,
|
||||
workProductIdFromArtifactReviewDocumentKey,
|
||||
type AttachmentArtifactWorkProductLike,
|
||||
} from "./markdown-work-products.js";
|
||||
export {
|
||||
ISSUE_WRITE_DENIAL_CODES,
|
||||
describeIssueWriteDenial,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,178 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX,
|
||||
artifactReviewDocumentKey,
|
||||
getAttachmentArtifactWorkProductMetadata,
|
||||
getMarkdownWorkProductAttachmentMetadata,
|
||||
isArtifactReviewDocumentKey,
|
||||
isMarkdownArtifactWorkProduct,
|
||||
isMarkdownAttachmentContent,
|
||||
workProductIdFromArtifactReviewDocumentKey,
|
||||
} from "./markdown-work-products.js";
|
||||
|
||||
const ATTACHMENT_ID = "00000000-0000-4000-8000-000000000001";
|
||||
const WORK_PRODUCT_ID = "11111111-2222-4333-8444-555555555555";
|
||||
|
||||
function attachmentMetadata(overrides: Record<string, unknown> = {}) {
|
||||
const contentPath = `/api/attachments/${ATTACHMENT_ID}/content`;
|
||||
return {
|
||||
attachmentId: ATTACHMENT_ID,
|
||||
contentType: "text/markdown",
|
||||
byteSize: 128,
|
||||
contentPath,
|
||||
openPath: contentPath,
|
||||
downloadPath: `${contentPath}?download=1`,
|
||||
originalFilename: "notes.md",
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function workProduct(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
type: "artifact" as const,
|
||||
provider: "paperclip",
|
||||
metadata: attachmentMetadata(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("isMarkdownAttachmentContent", () => {
|
||||
it.each([
|
||||
"text/markdown",
|
||||
"text/x-markdown",
|
||||
"application/markdown",
|
||||
"application/x-markdown",
|
||||
])("accepts %s regardless of filename", (contentType) => {
|
||||
expect(isMarkdownAttachmentContent({ contentType, originalFilename: null })).toBe(true);
|
||||
});
|
||||
|
||||
it("ignores MIME parameters and case", () => {
|
||||
expect(
|
||||
isMarkdownAttachmentContent({
|
||||
contentType: "Text/Markdown; charset=utf-8",
|
||||
originalFilename: null,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
"text/plain",
|
||||
"application/octet-stream",
|
||||
"binary/octet-stream",
|
||||
"application/x-binary",
|
||||
])("accepts a .md filename when the content type is %s", (contentType) => {
|
||||
expect(isMarkdownAttachmentContent({ contentType, originalFilename: "README.md" })).toBe(true);
|
||||
});
|
||||
|
||||
it("accepts a .markdown filename with a generic content type", () => {
|
||||
expect(
|
||||
isMarkdownAttachmentContent({
|
||||
contentType: "application/octet-stream",
|
||||
originalFilename: "guide.markdown",
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects .mdx files", () => {
|
||||
expect(
|
||||
isMarkdownAttachmentContent({
|
||||
contentType: "application/octet-stream",
|
||||
originalFilename: "page.mdx",
|
||||
}),
|
||||
).toBe(false);
|
||||
expect(
|
||||
isMarkdownAttachmentContent({ contentType: "text/plain", originalFilename: "page.mdx" }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects markdown filenames with non-generic content types", () => {
|
||||
expect(
|
||||
isMarkdownAttachmentContent({ contentType: "text/html", originalFilename: "notes.md" }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects non-markdown filenames with generic content types", () => {
|
||||
expect(
|
||||
isMarkdownAttachmentContent({ contentType: "text/plain", originalFilename: "notes.txt" }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects a missing content type", () => {
|
||||
expect(isMarkdownAttachmentContent({ contentType: null, originalFilename: "notes.md" })).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("getAttachmentArtifactWorkProductMetadata", () => {
|
||||
it("returns canonical metadata for a valid paperclip artifact work product", () => {
|
||||
const metadata = getAttachmentArtifactWorkProductMetadata(workProduct());
|
||||
expect(metadata?.attachmentId).toBe(ATTACHMENT_ID);
|
||||
});
|
||||
|
||||
it("returns null for non-artifact work products", () => {
|
||||
expect(getAttachmentArtifactWorkProductMetadata(workProduct({ type: "document" }))).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for non-paperclip providers", () => {
|
||||
expect(getAttachmentArtifactWorkProductMetadata(workProduct({ provider: "github" }))).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when metadata is missing or non-canonical", () => {
|
||||
expect(getAttachmentArtifactWorkProductMetadata(workProduct({ metadata: null }))).toBeNull();
|
||||
expect(
|
||||
getAttachmentArtifactWorkProductMetadata(
|
||||
workProduct({
|
||||
metadata: attachmentMetadata({ openPath: "https://evil.example/content" }),
|
||||
}),
|
||||
),
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("getMarkdownWorkProductAttachmentMetadata", () => {
|
||||
it("returns metadata only when the attachment is markdown-eligible", () => {
|
||||
expect(getMarkdownWorkProductAttachmentMetadata(workProduct())).not.toBeNull();
|
||||
expect(
|
||||
getMarkdownWorkProductAttachmentMetadata(
|
||||
workProduct({
|
||||
metadata: attachmentMetadata({ contentType: "application/pdf", originalFilename: "a.pdf" }),
|
||||
}),
|
||||
),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("backs the boolean eligibility helper", () => {
|
||||
expect(isMarkdownArtifactWorkProduct(workProduct())).toBe(true);
|
||||
expect(
|
||||
isMarkdownArtifactWorkProduct(
|
||||
workProduct({
|
||||
metadata: attachmentMetadata({ contentType: "image/png", originalFilename: "a.png" }),
|
||||
}),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("artifact review document keys", () => {
|
||||
it("builds a deterministic key that fits the 64-character document key limit", () => {
|
||||
const key = artifactReviewDocumentKey(WORK_PRODUCT_ID);
|
||||
expect(key).toBe(`artifact-review-${WORK_PRODUCT_ID}`);
|
||||
expect(key.length).toBeLessThanOrEqual(64);
|
||||
expect(key).toMatch(/^[a-z0-9][a-z0-9_-]*$/);
|
||||
});
|
||||
|
||||
it("identifies proxy review-document keys", () => {
|
||||
expect(isArtifactReviewDocumentKey(artifactReviewDocumentKey(WORK_PRODUCT_ID))).toBe(true);
|
||||
expect(isArtifactReviewDocumentKey("plan")).toBe(false);
|
||||
expect(isArtifactReviewDocumentKey("notes")).toBe(false);
|
||||
expect(isArtifactReviewDocumentKey(ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX)).toBe(false);
|
||||
});
|
||||
|
||||
it("round-trips the work product id", () => {
|
||||
expect(
|
||||
workProductIdFromArtifactReviewDocumentKey(artifactReviewDocumentKey(WORK_PRODUCT_ID)),
|
||||
).toBe(WORK_PRODUCT_ID);
|
||||
expect(workProductIdFromArtifactReviewDocumentKey("plan")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,99 @@
|
|||
import type {
|
||||
AttachmentArtifactWorkProductMetadata,
|
||||
IssueWorkProduct,
|
||||
} from "./types/work-product.js";
|
||||
import { attachmentArtifactWorkProductMetadataSchema } from "./validators/work-product.js";
|
||||
|
||||
/**
|
||||
* Shared classification for bridging attachment-backed Markdown work products
|
||||
* into the issue document review experience. Server materialization and UI
|
||||
* presentation must agree on eligibility, so both sides use these helpers.
|
||||
*/
|
||||
|
||||
export const MARKDOWN_ATTACHMENT_CONTENT_TYPES = [
|
||||
"text/markdown",
|
||||
"text/x-markdown",
|
||||
"application/markdown",
|
||||
"application/x-markdown",
|
||||
] as const;
|
||||
|
||||
const MARKDOWN_ATTACHMENT_CONTENT_TYPE_SET = new Set<string>(MARKDOWN_ATTACHMENT_CONTENT_TYPES);
|
||||
|
||||
const MARKDOWN_FALLBACK_CONTENT_TYPES = new Set([
|
||||
"text/plain",
|
||||
"application/octet-stream",
|
||||
"binary/octet-stream",
|
||||
"application/x-binary",
|
||||
]);
|
||||
|
||||
const MARKDOWN_FILENAME_EXTENSIONS = [".md", ".markdown"];
|
||||
|
||||
/**
|
||||
* Maximum attachment size eligible for review-document materialization.
|
||||
* Matches the issue-document body limit: a UTF-8 payload of at most this many
|
||||
* bytes always fits the 524288-character body cap.
|
||||
*/
|
||||
export const MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES = 512 * 1024;
|
||||
|
||||
export const ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX = "artifact-review-";
|
||||
|
||||
function normalizeAttachmentContentType(contentType: string | null | undefined): string {
|
||||
return (contentType ?? "").toLowerCase().split(";")[0]?.trim() ?? "";
|
||||
}
|
||||
|
||||
export function isMarkdownAttachmentContent(input: {
|
||||
contentType: string | null | undefined;
|
||||
originalFilename?: string | null;
|
||||
}): boolean {
|
||||
const contentType = normalizeAttachmentContentType(input.contentType);
|
||||
if (MARKDOWN_ATTACHMENT_CONTENT_TYPE_SET.has(contentType)) return true;
|
||||
const filename = (input.originalFilename ?? "").toLowerCase();
|
||||
if (!MARKDOWN_FILENAME_EXTENSIONS.some((extension) => filename.endsWith(extension))) {
|
||||
return false;
|
||||
}
|
||||
return MARKDOWN_FALLBACK_CONTENT_TYPES.has(contentType);
|
||||
}
|
||||
|
||||
export type AttachmentArtifactWorkProductLike = Pick<
|
||||
IssueWorkProduct,
|
||||
"type" | "provider" | "metadata"
|
||||
>;
|
||||
|
||||
export function getAttachmentArtifactWorkProductMetadata(
|
||||
workProduct: AttachmentArtifactWorkProductLike,
|
||||
): AttachmentArtifactWorkProductMetadata | null {
|
||||
if (workProduct.type !== "artifact" || workProduct.provider !== "paperclip") return null;
|
||||
if (!workProduct.metadata) return null;
|
||||
const parsed = attachmentArtifactWorkProductMetadataSchema.safeParse(workProduct.metadata);
|
||||
return parsed.success ? parsed.data : null;
|
||||
}
|
||||
|
||||
export function getMarkdownWorkProductAttachmentMetadata(
|
||||
workProduct: AttachmentArtifactWorkProductLike,
|
||||
): AttachmentArtifactWorkProductMetadata | null {
|
||||
const metadata = getAttachmentArtifactWorkProductMetadata(workProduct);
|
||||
if (!metadata) return null;
|
||||
return isMarkdownAttachmentContent(metadata) ? metadata : null;
|
||||
}
|
||||
|
||||
export function isMarkdownArtifactWorkProduct(
|
||||
workProduct: AttachmentArtifactWorkProductLike,
|
||||
): boolean {
|
||||
return getMarkdownWorkProductAttachmentMetadata(workProduct) !== null;
|
||||
}
|
||||
|
||||
export function artifactReviewDocumentKey(workProductId: string): string {
|
||||
return `${ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX}${workProductId}`;
|
||||
}
|
||||
|
||||
export function isArtifactReviewDocumentKey(key: string): boolean {
|
||||
return (
|
||||
key.startsWith(ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX) &&
|
||||
key.length > ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX.length
|
||||
);
|
||||
}
|
||||
|
||||
export function workProductIdFromArtifactReviewDocumentKey(key: string): string | null {
|
||||
if (!isArtifactReviewDocumentKey(key)) return null;
|
||||
return key.slice(ARTIFACT_REVIEW_DOCUMENT_KEY_PREFIX.length);
|
||||
}
|
||||
|
|
@ -0,0 +1,334 @@
|
|||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { StorageService } from "../storage/types.js";
|
||||
|
||||
const ISSUE_ID = "11111111-1111-4111-8111-111111111111";
|
||||
const WORK_PRODUCT_ID = "22222222-2222-4222-8222-222222222222";
|
||||
|
||||
const mockIssueService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
getByIdentifier: vi.fn(),
|
||||
getAttachmentById: vi.fn(),
|
||||
}));
|
||||
const mockCompanyService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
}));
|
||||
const mockWorkProductService = vi.hoisted(() => ({
|
||||
createForIssue: vi.fn(),
|
||||
getById: vi.fn(),
|
||||
update: vi.fn(),
|
||||
}));
|
||||
const mockAccessService = vi.hoisted(() => ({
|
||||
decide: vi.fn(async () => ({
|
||||
allowed: true,
|
||||
explanation: "Allowed by test mock",
|
||||
})),
|
||||
canUser: vi.fn(),
|
||||
hasPermission: vi.fn(),
|
||||
}));
|
||||
const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
const mockEnsureForWorkProduct = vi.hoisted(() => vi.fn());
|
||||
const mockSyncDocumentSafely = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
|
||||
function registerRouteMocks() {
|
||||
vi.doMock("@paperclipai/shared/telemetry", () => ({
|
||||
trackAgentTaskCompleted: vi.fn(),
|
||||
trackErrorHandlerCrash: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.doMock("../telemetry.js", () => ({
|
||||
getTelemetryClient: vi.fn(() => ({ track: vi.fn() })),
|
||||
}));
|
||||
|
||||
vi.doMock("../services/issues.js", () => ({
|
||||
issueService: () => mockIssueService,
|
||||
}));
|
||||
|
||||
vi.doMock("../services/activity-log.js", () => ({
|
||||
logActivity: mockLogActivity,
|
||||
}));
|
||||
|
||||
vi.doMock("../services/artifact-review-documents.js", () => ({
|
||||
artifactReviewDocumentService: () => ({
|
||||
ensureForWorkProduct: mockEnsureForWorkProduct,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.doMock("../services/external-objects.js", () => ({
|
||||
externalObjectService: () => ({
|
||||
syncDocumentSafely: mockSyncDocumentSafely,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.doMock("../services/index.js", () => ({
|
||||
accessService: () => mockAccessService,
|
||||
agentService: () => ({
|
||||
getById: vi.fn(),
|
||||
}),
|
||||
companySkillService: () => ({}),
|
||||
companyService: () => mockCompanyService,
|
||||
documentAnnotationService: () => ({ remapOpenThreadsForDocument: async () => [] }),
|
||||
documentService: () => ({}),
|
||||
executionWorkspaceService: () => ({}),
|
||||
feedbackService: () => ({
|
||||
listIssueVotesForUser: vi.fn(async () => []),
|
||||
saveIssueVote: vi.fn(async () => ({ vote: null, consentEnabledNow: false, sharingEnabled: false })),
|
||||
}),
|
||||
goalService: () => ({}),
|
||||
heartbeatService: () => ({
|
||||
wakeup: vi.fn(async () => undefined),
|
||||
reportRunActivity: vi.fn(async () => undefined),
|
||||
getRun: vi.fn(async () => null),
|
||||
getActiveRunForAgent: vi.fn(async () => null),
|
||||
cancelRun: vi.fn(async () => null),
|
||||
}),
|
||||
instanceSettingsService: () => ({
|
||||
get: vi.fn(async () => ({
|
||||
id: "instance-settings-1",
|
||||
general: {
|
||||
censorUsernameInLogs: false,
|
||||
feedbackDataSharingPreference: "prompt",
|
||||
},
|
||||
})),
|
||||
listCompanyIds: vi.fn(async () => ["company-1"]),
|
||||
}),
|
||||
issueApprovalService: () => ({}),
|
||||
issueReferenceService: () => ({
|
||||
deleteDocumentSource: async () => undefined,
|
||||
diffIssueReferenceSummary: () => ({
|
||||
addedReferencedIssues: [],
|
||||
removedReferencedIssues: [],
|
||||
currentReferencedIssues: [],
|
||||
}),
|
||||
emptySummary: () => ({ outbound: [], inbound: [] }),
|
||||
listIssueReferenceSummary: async () => ({ outbound: [], inbound: [] }),
|
||||
syncComment: async () => undefined,
|
||||
syncDocument: async () => undefined,
|
||||
syncIssue: async () => undefined,
|
||||
}),
|
||||
issueThreadInteractionService: () => ({
|
||||
listForIssue: vi.fn(async () => []),
|
||||
expireRequestConfirmationsSupersededByComment: vi.fn(async () => []),
|
||||
expireStaleRequestConfirmationsForIssueDocument: vi.fn(async () => []),
|
||||
}),
|
||||
issueRecoveryActionService: () => ({
|
||||
getActiveForIssue: vi.fn(async () => null),
|
||||
listActiveForIssues: vi.fn(async () => new Map()),
|
||||
}),
|
||||
issueService: () => mockIssueService,
|
||||
logActivity: mockLogActivity,
|
||||
projectService: () => ({}),
|
||||
routineService: () => ({
|
||||
syncRunStatusForIssue: vi.fn(async () => undefined),
|
||||
}),
|
||||
workProductService: () => mockWorkProductService,
|
||||
}));
|
||||
}
|
||||
|
||||
function createStorageService(): StorageService {
|
||||
return {
|
||||
provider: "local_disk",
|
||||
putFile: vi.fn(),
|
||||
getObject: vi.fn(),
|
||||
headObject: vi.fn(),
|
||||
deleteObject: vi.fn(),
|
||||
} as unknown as StorageService;
|
||||
}
|
||||
|
||||
async function createApp(options?: { companyIds?: string[] }) {
|
||||
const [{ errorHandler }, { issueRoutes }] = await Promise.all([
|
||||
vi.importActual<typeof import("../middleware/index.js")>("../middleware/index.js"),
|
||||
vi.importActual<typeof import("../routes/issues.js")>("../routes/issues.js"),
|
||||
]);
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
(req as any).actor = {
|
||||
type: "board",
|
||||
userId: "local-board",
|
||||
companyIds: options?.companyIds ?? ["company-1"],
|
||||
source: "local_implicit",
|
||||
isInstanceAdmin: false,
|
||||
};
|
||||
next();
|
||||
});
|
||||
app.use("/api", issueRoutes({} as any, createStorageService()));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
||||
function makeIssue() {
|
||||
return {
|
||||
id: ISSUE_ID,
|
||||
companyId: "company-1",
|
||||
identifier: "PAP-1",
|
||||
projectId: null,
|
||||
status: "in_progress",
|
||||
assigneeAgentId: null,
|
||||
};
|
||||
}
|
||||
|
||||
function makeWorkProduct(overrides: Record<string, unknown> = {}) {
|
||||
const attachmentId = "55555555-5555-4555-8555-555555555555";
|
||||
const contentPath = `/api/attachments/${attachmentId}/content`;
|
||||
return {
|
||||
id: WORK_PRODUCT_ID,
|
||||
companyId: "company-1",
|
||||
issueId: ISSUE_ID,
|
||||
type: "artifact",
|
||||
provider: "paperclip",
|
||||
title: "Verification report",
|
||||
metadata: {
|
||||
attachmentId,
|
||||
contentType: "text/markdown",
|
||||
byteSize: 128,
|
||||
contentPath,
|
||||
openPath: contentPath,
|
||||
downloadPath: `${contentPath}?download=1`,
|
||||
originalFilename: "verification-report.md",
|
||||
},
|
||||
createdByRunId: null,
|
||||
sourceTrust: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeDocument(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: "33333333-3333-4333-8333-333333333333",
|
||||
companyId: "company-1",
|
||||
issueId: ISSUE_ID,
|
||||
key: `artifact-review-${WORK_PRODUCT_ID}`,
|
||||
title: "Verification report",
|
||||
format: "markdown",
|
||||
body: "# Report",
|
||||
latestRevisionId: "44444444-4444-4444-8444-444444444444",
|
||||
latestRevisionNumber: 1,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("work product review-document route", () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules();
|
||||
vi.clearAllMocks();
|
||||
registerRouteMocks();
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue());
|
||||
mockWorkProductService.getById.mockResolvedValue(makeWorkProduct());
|
||||
});
|
||||
|
||||
it("returns 404 when the work product belongs to another issue", async () => {
|
||||
mockWorkProductService.getById.mockResolvedValue(
|
||||
makeWorkProduct({ issueId: "55555555-5555-4555-8555-555555555555" }),
|
||||
);
|
||||
|
||||
const app = await createApp();
|
||||
const res = await request(app).post(
|
||||
`/api/issues/${ISSUE_ID}/work-products/${WORK_PRODUCT_ID}/review-document`,
|
||||
);
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(mockEnsureForWorkProduct).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns 404 when the work product does not exist", async () => {
|
||||
mockWorkProductService.getById.mockResolvedValue(null);
|
||||
|
||||
const app = await createApp();
|
||||
const res = await request(app).post(
|
||||
`/api/issues/${ISSUE_ID}/work-products/${WORK_PRODUCT_ID}/review-document`,
|
||||
);
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(mockEnsureForWorkProduct).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("materializes the review document and logs document creation", async () => {
|
||||
mockEnsureForWorkProduct.mockResolvedValue({
|
||||
document: makeDocument(),
|
||||
created: true,
|
||||
revisionChanged: true,
|
||||
remappedAnnotations: [],
|
||||
});
|
||||
|
||||
const app = await createApp();
|
||||
const res = await request(app).post(
|
||||
`/api/issues/${ISSUE_ID}/work-products/${WORK_PRODUCT_ID}/review-document`,
|
||||
);
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(res.body.key).toBe(`artifact-review-${WORK_PRODUCT_ID}`);
|
||||
expect(mockEnsureForWorkProduct).toHaveBeenCalledWith({
|
||||
issue: { id: ISSUE_ID, companyId: "company-1" },
|
||||
workProduct: expect.objectContaining({ id: WORK_PRODUCT_ID }),
|
||||
});
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({
|
||||
action: "issue.document_created",
|
||||
details: expect.objectContaining({
|
||||
workProductId: WORK_PRODUCT_ID,
|
||||
artifactReviewDocument: true,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expect(mockSyncDocumentSafely).toHaveBeenCalledWith(makeDocument().id);
|
||||
});
|
||||
|
||||
it("returns 200 without side effects when the document is already current", async () => {
|
||||
mockEnsureForWorkProduct.mockResolvedValue({
|
||||
document: makeDocument(),
|
||||
created: false,
|
||||
revisionChanged: false,
|
||||
remappedAnnotations: [],
|
||||
});
|
||||
|
||||
const app = await createApp();
|
||||
const res = await request(app).post(
|
||||
`/api/issues/${ISSUE_ID}/work-products/${WORK_PRODUCT_ID}/review-document`,
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.id).toBe(makeDocument().id);
|
||||
expect(mockLogActivity).not.toHaveBeenCalled();
|
||||
expect(mockSyncDocumentSafely).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("passes typed materialization errors through to the response", async () => {
|
||||
const { HttpError } = await vi.importActual<typeof import("../errors.js")>("../errors.js");
|
||||
mockEnsureForWorkProduct.mockRejectedValue(
|
||||
new HttpError(415, "Work product attachment is not Markdown"),
|
||||
);
|
||||
|
||||
const app = await createApp();
|
||||
const res = await request(app).post(
|
||||
`/api/issues/${ISSUE_ID}/work-products/${WORK_PRODUCT_ID}/review-document`,
|
||||
);
|
||||
|
||||
expect(res.status).toBe(415);
|
||||
expect(res.body.error).toBe("Work product attachment is not Markdown");
|
||||
});
|
||||
|
||||
it("resynchronizes an eligible review document after a title-only work product update", async () => {
|
||||
const renamed = makeWorkProduct({ title: "Renamed verification report" });
|
||||
mockWorkProductService.update.mockResolvedValue(renamed);
|
||||
mockEnsureForWorkProduct.mockResolvedValue({
|
||||
document: makeDocument({ title: "Renamed verification report", latestRevisionNumber: 2 }),
|
||||
created: false,
|
||||
revisionChanged: true,
|
||||
remappedAnnotations: [],
|
||||
});
|
||||
|
||||
const app = await createApp();
|
||||
const res = await request(app)
|
||||
.patch(`/api/work-products/${WORK_PRODUCT_ID}`)
|
||||
.send({ title: "Renamed verification report" });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockEnsureForWorkProduct).toHaveBeenCalledWith({
|
||||
issue: { id: ISSUE_ID, companyId: "company-1" },
|
||||
workProduct: renamed,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,403 @@
|
|||
import { randomUUID } from "node:crypto";
|
||||
import { Readable } from "node:stream";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
agents,
|
||||
assets,
|
||||
companies,
|
||||
createDb,
|
||||
documentRevisions,
|
||||
documents,
|
||||
heartbeatRuns,
|
||||
issueAttachments,
|
||||
issueDocuments,
|
||||
issues,
|
||||
} from "@paperclipai/db";
|
||||
import { artifactReviewDocumentKey } from "@paperclipai/shared";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { artifactReviewDocumentService } from "../services/artifact-review-documents.js";
|
||||
import { documentService } from "../services/documents.js";
|
||||
import { HttpError } from "../errors.js";
|
||||
import type { StorageService } from "../storage/types.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
if (!embeddedPostgresSupport.supported) {
|
||||
console.warn(
|
||||
`Skipping embedded Postgres artifact review document tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`,
|
||||
);
|
||||
}
|
||||
|
||||
function createStorageService(files: Record<string, Buffer> = {}): StorageService {
|
||||
return {
|
||||
provider: "local_disk",
|
||||
putFile: vi.fn(),
|
||||
getObject: vi.fn(async (_companyId, objectKey, options) => {
|
||||
const body = files[objectKey] ?? Buffer.alloc(0);
|
||||
const range = options?.range;
|
||||
const ranged = range ? body.subarray(range.start, range.end + 1) : body;
|
||||
return {
|
||||
stream: Readable.from(ranged),
|
||||
contentType: "text/markdown",
|
||||
contentLength: ranged.length,
|
||||
};
|
||||
}),
|
||||
headObject: vi.fn(),
|
||||
deleteObject: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
describeEmbeddedPostgres("artifactReviewDocumentService", () => {
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-artifact-review-docs-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
await db.delete(documentRevisions);
|
||||
await db.delete(issueDocuments);
|
||||
await db.delete(documents);
|
||||
await db.delete(issueAttachments);
|
||||
await db.delete(assets);
|
||||
await db.delete(issues);
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(agents);
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
async function seedFixture(options: {
|
||||
body?: Buffer;
|
||||
contentType?: string;
|
||||
originalFilename?: string | null;
|
||||
byteSize?: number;
|
||||
} = {}) {
|
||||
const companyId = randomUUID();
|
||||
const issueId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
const assetId = randomUUID();
|
||||
const attachmentId = randomUUID();
|
||||
const workProductId = randomUUID();
|
||||
const body = options.body ?? Buffer.from("# Report\n\nHello **markdown**.\n", "utf8");
|
||||
const contentType = options.contentType ?? "text/markdown";
|
||||
const originalFilename = options.originalFilename === undefined ? "report.md" : options.originalFilename;
|
||||
const objectKey = `issues/${issueId}/${assetId}`;
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await db.insert(issues).values({
|
||||
id: issueId,
|
||||
companyId,
|
||||
identifier: "PAP-1534",
|
||||
title: "Bridge markdown work products",
|
||||
description: null,
|
||||
status: "in_progress",
|
||||
priority: "medium",
|
||||
});
|
||||
await db.insert(agents).values({
|
||||
id: agentId,
|
||||
companyId,
|
||||
name: "ReportWriter",
|
||||
role: "engineer",
|
||||
status: "idle",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
await db.insert(assets).values({
|
||||
id: assetId,
|
||||
companyId,
|
||||
provider: "local_disk",
|
||||
objectKey,
|
||||
contentType,
|
||||
byteSize: options.byteSize ?? body.length,
|
||||
sha256: `sha256-${assetId}`,
|
||||
originalFilename,
|
||||
createdByAgentId: agentId,
|
||||
});
|
||||
await db.insert(issueAttachments).values({
|
||||
id: attachmentId,
|
||||
companyId,
|
||||
issueId,
|
||||
assetId,
|
||||
});
|
||||
|
||||
const contentPath = `/api/attachments/${attachmentId}/content`;
|
||||
const workProduct = {
|
||||
id: workProductId,
|
||||
companyId,
|
||||
issueId,
|
||||
type: "artifact" as const,
|
||||
provider: "paperclip",
|
||||
metadata: {
|
||||
attachmentId,
|
||||
contentType,
|
||||
byteSize: options.byteSize ?? body.length,
|
||||
contentPath,
|
||||
openPath: contentPath,
|
||||
downloadPath: `${contentPath}?download=1`,
|
||||
originalFilename,
|
||||
},
|
||||
title: "Verification report",
|
||||
createdByRunId: null,
|
||||
sourceTrust: null,
|
||||
};
|
||||
|
||||
const files: Record<string, Buffer> = { [objectKey]: body };
|
||||
const storage = createStorageService(files);
|
||||
const svc = artifactReviewDocumentService(db, storage);
|
||||
return { companyId, issueId, agentId, assetId, attachmentId, workProduct, files, objectKey, svc };
|
||||
}
|
||||
|
||||
it("materializes a review document with promoted attribution", async () => {
|
||||
const fixture = await seedFixture();
|
||||
const result = await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
});
|
||||
|
||||
expect(result.created).toBe(true);
|
||||
expect(result.revisionChanged).toBe(true);
|
||||
expect(result.document.key).toBe(artifactReviewDocumentKey(fixture.workProduct.id));
|
||||
expect(result.document.title).toBe("Verification report");
|
||||
expect(result.document.latestRevisionNumber).toBe(1);
|
||||
expect(result.document.latestRevisionId).toBeTruthy();
|
||||
expect(result.document.body).toBe("# Report\n\nHello **markdown**.\n");
|
||||
expect(result.document.createdByAgentId).toBe(fixture.agentId);
|
||||
expect(result.document.createdByUserId).toBeNull();
|
||||
|
||||
const listed = await documentService(db).getIssueDocumentByKey(
|
||||
fixture.issueId,
|
||||
artifactReviewDocumentKey(fixture.workProduct.id),
|
||||
);
|
||||
expect(listed?.id).toBe(result.document.id);
|
||||
});
|
||||
|
||||
it("is idempotent for unchanged content", async () => {
|
||||
const fixture = await seedFixture();
|
||||
const first = await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
});
|
||||
const second = await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
});
|
||||
|
||||
expect(second.created).toBe(false);
|
||||
expect(second.revisionChanged).toBe(false);
|
||||
expect(second.document.id).toBe(first.document.id);
|
||||
expect(second.document.latestRevisionNumber).toBe(1);
|
||||
|
||||
const revisions = await db.select().from(documentRevisions);
|
||||
expect(revisions).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("converges concurrent ensures onto one document", async () => {
|
||||
const fixture = await seedFixture();
|
||||
const input = {
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
};
|
||||
const [first, second] = await Promise.all([
|
||||
fixture.svc.ensureForWorkProduct(input),
|
||||
fixture.svc.ensureForWorkProduct(input),
|
||||
]);
|
||||
|
||||
expect(first.document.id).toBe(second.document.id);
|
||||
const links = await db.select().from(issueDocuments);
|
||||
expect(links).toHaveLength(1);
|
||||
const revisions = await db.select().from(documentRevisions);
|
||||
expect(revisions).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("writes a new revision when the attachment content changes", async () => {
|
||||
const fixture = await seedFixture();
|
||||
const first = await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
});
|
||||
|
||||
fixture.files[fixture.objectKey] = Buffer.from("# Report v2\n", "utf8");
|
||||
const second = await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
});
|
||||
|
||||
expect(second.created).toBe(false);
|
||||
expect(second.revisionChanged).toBe(true);
|
||||
expect(second.document.id).toBe(first.document.id);
|
||||
expect(second.document.latestRevisionNumber).toBe(2);
|
||||
expect(second.document.body).toBe("# Report v2\n");
|
||||
expect(Array.isArray(second.remappedAnnotations)).toBe(true);
|
||||
});
|
||||
|
||||
it("synchronizes title and run provenance when the attachment body is unchanged", async () => {
|
||||
const fixture = await seedFixture();
|
||||
const first = await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
});
|
||||
|
||||
const renamed = await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: { ...fixture.workProduct, title: "Renamed verification report" },
|
||||
});
|
||||
expect(renamed.created).toBe(false);
|
||||
expect(renamed.revisionChanged).toBe(true);
|
||||
expect(renamed.document.id).toBe(first.document.id);
|
||||
expect(renamed.document.title).toBe("Renamed verification report");
|
||||
expect(renamed.document.latestRevisionNumber).toBe(2);
|
||||
|
||||
const runId = randomUUID();
|
||||
await db.insert(heartbeatRuns).values({
|
||||
id: runId,
|
||||
companyId: fixture.companyId,
|
||||
agentId: fixture.agentId,
|
||||
status: "succeeded",
|
||||
});
|
||||
const reattributed = await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: {
|
||||
...fixture.workProduct,
|
||||
title: "Renamed verification report",
|
||||
createdByRunId: runId,
|
||||
},
|
||||
});
|
||||
expect(reattributed.revisionChanged).toBe(true);
|
||||
expect(reattributed.document.latestRevisionNumber).toBe(3);
|
||||
|
||||
const latestRevision = await db
|
||||
.select()
|
||||
.from(documentRevisions)
|
||||
.then((rows) => rows.find((row) => row.id === reattributed.document.latestRevisionId));
|
||||
expect(latestRevision?.createdByRunId).toBe(runId);
|
||||
|
||||
const unchanged = await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: {
|
||||
...fixture.workProduct,
|
||||
title: "Renamed verification report",
|
||||
createdByRunId: runId,
|
||||
},
|
||||
});
|
||||
expect(unchanged.revisionChanged).toBe(false);
|
||||
expect(unchanged.document.latestRevisionNumber).toBe(3);
|
||||
});
|
||||
|
||||
it("rejects work products that are not attachment-backed Paperclip artifacts", async () => {
|
||||
const fixture = await seedFixture();
|
||||
await expect(
|
||||
fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: { ...fixture.workProduct, provider: "github" },
|
||||
}),
|
||||
).rejects.toMatchObject({ status: 422 });
|
||||
await expect(
|
||||
fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: { ...fixture.workProduct, metadata: { attachmentId: "not-a-uuid" } },
|
||||
}),
|
||||
).rejects.toMatchObject({ status: 422 });
|
||||
});
|
||||
|
||||
it("rejects non-Markdown attachments with 415", async () => {
|
||||
const fixture = await seedFixture({ contentType: "application/pdf", originalFilename: "report.pdf" });
|
||||
await expect(
|
||||
fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
}),
|
||||
).rejects.toMatchObject({ status: 415 });
|
||||
});
|
||||
|
||||
it("rejects mismatched issues and cross-issue attachments", async () => {
|
||||
const fixture = await seedFixture();
|
||||
await expect(
|
||||
fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: randomUUID(), companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
}),
|
||||
).rejects.toMatchObject({ status: 404 });
|
||||
|
||||
const otherIssueId = randomUUID();
|
||||
await db.insert(issues).values({
|
||||
id: otherIssueId,
|
||||
companyId: fixture.companyId,
|
||||
identifier: "PAP-1535",
|
||||
title: "Other issue",
|
||||
description: null,
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
});
|
||||
await expect(
|
||||
fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: otherIssueId, companyId: fixture.companyId },
|
||||
workProduct: { ...fixture.workProduct, issueId: otherIssueId },
|
||||
}),
|
||||
).rejects.toMatchObject({ status: 422 });
|
||||
});
|
||||
|
||||
it("rejects oversized attachments with 413", async () => {
|
||||
const fixture = await seedFixture({ byteSize: 512 * 1024 + 1 });
|
||||
await expect(
|
||||
fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
}),
|
||||
).rejects.toMatchObject({ status: 413 });
|
||||
});
|
||||
|
||||
it("rejects invalid UTF-8 content with 422", async () => {
|
||||
const fixture = await seedFixture({ body: Buffer.from([0x23, 0x20, 0xff, 0xfe, 0x00]) });
|
||||
await expect(
|
||||
fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
}),
|
||||
).rejects.toMatchObject({ status: 422 });
|
||||
});
|
||||
|
||||
it("surfaces a conflict when the review document is locked and content changed", async () => {
|
||||
const fixture = await seedFixture();
|
||||
await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
});
|
||||
await documentService(db).lockIssueDocument({
|
||||
issueId: fixture.issueId,
|
||||
key: artifactReviewDocumentKey(fixture.workProduct.id),
|
||||
lockedByUserId: "user-board",
|
||||
});
|
||||
|
||||
// Unchanged content still resolves without a write.
|
||||
const unchanged = await fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
});
|
||||
expect(unchanged.revisionChanged).toBe(false);
|
||||
|
||||
fixture.files[fixture.objectKey] = Buffer.from("# Locked update\n", "utf8");
|
||||
await expect(
|
||||
fixture.svc.ensureForWorkProduct({
|
||||
issue: { id: fixture.issueId, companyId: fixture.companyId },
|
||||
workProduct: fixture.workProduct,
|
||||
}),
|
||||
).rejects.toSatisfy((error: unknown) => error instanceof HttpError && error.status === 409);
|
||||
});
|
||||
});
|
||||
|
|
@ -515,6 +515,36 @@ describe("issue attachment routes", () => {
|
|||
expect(res.headers["x-content-type-options"]).toBe("nosniff");
|
||||
});
|
||||
|
||||
it("declares utf-8 for inline markdown attachments", async () => {
|
||||
const storage = createStorageService(Buffer.from("# Hello\n"));
|
||||
mockIssueService.getAttachmentById.mockResolvedValue({
|
||||
...makeAttachment("text/markdown", "notes.md"),
|
||||
byteSize: 8,
|
||||
});
|
||||
|
||||
const app = await createApp(storage);
|
||||
const res = await request(app).get("/api/attachments/attachment-1/content");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.headers["content-type"]).toBe("text/markdown; charset=utf-8");
|
||||
expect(res.headers["x-content-type-options"]).toBe("nosniff");
|
||||
});
|
||||
|
||||
it("keeps the charset declaration on forced markdown downloads", async () => {
|
||||
const storage = createStorageService(Buffer.from("# Hello\n"));
|
||||
mockIssueService.getAttachmentById.mockResolvedValue({
|
||||
...makeAttachment("text/markdown", "notes.md"),
|
||||
byteSize: 8,
|
||||
});
|
||||
|
||||
const app = await createApp(storage);
|
||||
const res = await request(app).get("/api/attachments/attachment-1/content?download=1");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.headers["content-type"]).toBe("text/markdown; charset=utf-8");
|
||||
expect(res.headers["content-disposition"]).toBe('attachment; filename="notes.md"');
|
||||
});
|
||||
|
||||
it("keeps image attachments inline for previews", async () => {
|
||||
const storage = createStorageService();
|
||||
mockIssueService.getAttachmentById.mockResolvedValue(makeAttachment("image/png", "preview.png"));
|
||||
|
|
|
|||
|
|
@ -210,6 +210,11 @@ describe("openapi routes", () => {
|
|||
},
|
||||
});
|
||||
expect(res.body.paths["/api/companies/{companyId}/folders"].post.responses["201"]).toBeDefined();
|
||||
expect(
|
||||
Object.keys(
|
||||
res.body.paths["/api/issues/{id}/work-products/{workProductId}/review-document"].post.responses,
|
||||
).sort(),
|
||||
).toEqual(["200", "201", "401", "403", "404", "409", "413", "415", "422"]);
|
||||
expect(
|
||||
res.body.paths["/api/issues/{id}/interactions/{interactionId}/withdraw"].post.summary,
|
||||
).toBe("Withdraw a pending issue thread interaction");
|
||||
|
|
|
|||
|
|
@ -29,6 +29,14 @@ export function conflict(message: string, details?: unknown) {
|
|||
return new HttpError(409, message, details);
|
||||
}
|
||||
|
||||
export function payloadTooLarge(message: string, details?: unknown) {
|
||||
return new HttpError(413, message, details);
|
||||
}
|
||||
|
||||
export function unsupportedMediaType(message: string, details?: unknown) {
|
||||
return new HttpError(415, message, details);
|
||||
}
|
||||
|
||||
export function unprocessable(message: string, details?: unknown) {
|
||||
return new HttpError(422, message, details);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -67,6 +67,8 @@ import {
|
|||
upsertIssueDocumentSchema,
|
||||
updateIssueSchema,
|
||||
isClosedIsolatedExecutionWorkspace,
|
||||
isMarkdownArtifactWorkProduct,
|
||||
isMarkdownAttachmentContent,
|
||||
isUuidLike,
|
||||
normalizeIssueIdentifier as normalizeIssueReferenceIdentifier,
|
||||
type CompactIssue,
|
||||
|
|
@ -138,6 +140,7 @@ import {
|
|||
routineService,
|
||||
workProductService,
|
||||
} from "../services/index.js";
|
||||
import { artifactReviewDocumentService } from "../services/artifact-review-documents.js";
|
||||
import { assertCanResolveProposal } from "../services/secret-proposal-authorization.js";
|
||||
import { buildDocumentReviewContext, buildPlanReviewContext } from "../services/plan-review-context.js";
|
||||
import {
|
||||
|
|
@ -2830,6 +2833,7 @@ export function issueRoutes(
|
|||
const executionWorkspacesSvc = executionWorkspaceServiceDirect(db);
|
||||
const workProductsSvc = workProductService(db);
|
||||
const documentsSvc = documentService(db);
|
||||
const artifactReviewDocumentsSvc = artifactReviewDocumentService(db, storage);
|
||||
const companySkillsSvc = companySkillService(db);
|
||||
const documentAnnotationsSvc = documentAnnotationService(db);
|
||||
const decisionTrainingSvc = decisionTrainingService(db);
|
||||
|
|
@ -7888,9 +7892,110 @@ export function issueRoutes(
|
|||
actor,
|
||||
workProductChanged: true,
|
||||
});
|
||||
await materializeArtifactReviewDocumentBestEffort({ issue, workProduct: product, actor });
|
||||
res.status(201).json(product);
|
||||
});
|
||||
|
||||
async function ensureArtifactReviewDocumentForWorkProduct(input: {
|
||||
issue: NonNullable<Awaited<ReturnType<typeof svc.getById>>>;
|
||||
workProduct: NonNullable<Awaited<ReturnType<typeof workProductsSvc.getById>>>;
|
||||
actor: ReturnType<typeof getActorInfo>;
|
||||
}) {
|
||||
const { issue, workProduct, actor } = input;
|
||||
const result = await artifactReviewDocumentsSvc.ensureForWorkProduct({
|
||||
issue: { id: issue.id, companyId: issue.companyId },
|
||||
workProduct,
|
||||
});
|
||||
if (!result.revisionChanged) return result;
|
||||
const doc = result.document;
|
||||
await issueReferencesSvc.syncDocument(doc.id);
|
||||
await externalObjectsSvc.syncDocumentSafely(doc.id);
|
||||
await logActivity(db, {
|
||||
companyId: issue.companyId,
|
||||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
agentId: actor.agentId,
|
||||
runId: actor.runId,
|
||||
agentApiKeyId: actor.agentApiKeyId,
|
||||
action: result.created ? "issue.document_created" : "issue.document_updated",
|
||||
entityType: "issue",
|
||||
entityId: issue.id,
|
||||
details: {
|
||||
key: doc.key,
|
||||
documentId: doc.id,
|
||||
title: doc.title,
|
||||
format: doc.format,
|
||||
revisionNumber: doc.latestRevisionNumber,
|
||||
workProductId: workProduct.id,
|
||||
artifactReviewDocument: true,
|
||||
},
|
||||
});
|
||||
for (const remap of result.remappedAnnotations) {
|
||||
await logActivity(db, {
|
||||
companyId: issue.companyId,
|
||||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
agentId: actor.agentId,
|
||||
runId: actor.runId,
|
||||
agentApiKeyId: actor.agentApiKeyId,
|
||||
action: "issue.document_annotation_remapped",
|
||||
entityType: "issue",
|
||||
entityId: issue.id,
|
||||
details: {
|
||||
key: doc.key,
|
||||
documentId: doc.id,
|
||||
threadId: remap.thread.id,
|
||||
revisionNumber: doc.latestRevisionNumber,
|
||||
anchorState: remap.thread.anchorState,
|
||||
anchorConfidence: remap.thread.anchorConfidence,
|
||||
snapshotId: remap.snapshot.id,
|
||||
},
|
||||
});
|
||||
}
|
||||
await revalidateActiveSourceRecoveryAfterCommittedWrite({
|
||||
issue,
|
||||
trigger: "document",
|
||||
actor,
|
||||
documentChanged: true,
|
||||
});
|
||||
return result;
|
||||
}
|
||||
|
||||
async function materializeArtifactReviewDocumentBestEffort(input: {
|
||||
issue: NonNullable<Awaited<ReturnType<typeof svc.getById>>>;
|
||||
workProduct: NonNullable<Awaited<ReturnType<typeof workProductsSvc.getById>>>;
|
||||
actor: ReturnType<typeof getActorInfo>;
|
||||
}) {
|
||||
if (!isMarkdownArtifactWorkProduct(input.workProduct)) return;
|
||||
try {
|
||||
await ensureArtifactReviewDocumentForWorkProduct(input);
|
||||
} catch (error) {
|
||||
// Work-product writes stay fail-open: raw open and download remain
|
||||
// available, and the explicit review-document endpoint is the retry path.
|
||||
logger.warn(
|
||||
{ err: error, issueId: input.issue.id, workProductId: input.workProduct.id },
|
||||
"markdown work product review-document materialization failed",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
router.post("/issues/:id/work-products/:workProductId/review-document", async (req, res) => {
|
||||
const id = req.params.id as string;
|
||||
const workProductId = req.params.workProductId as string;
|
||||
const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found");
|
||||
if (!issue) return;
|
||||
if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return;
|
||||
if (!(await assertDeliverableMutationAllowedByRunContext(req, res, issue))) return;
|
||||
const workProduct = await workProductsSvc.getById(workProductId);
|
||||
if (!workProduct || workProduct.issueId !== issue.id || workProduct.companyId !== issue.companyId) {
|
||||
res.status(404).json({ error: "Work product not found" });
|
||||
return;
|
||||
}
|
||||
const actor = getActorInfo(req);
|
||||
const result = await ensureArtifactReviewDocumentForWorkProduct({ issue, workProduct, actor });
|
||||
res.status(result.created ? 201 : 200).json(result.document);
|
||||
});
|
||||
|
||||
router.post("/issues/:id/low-trust/promotions", validate(promoteLowTrustOutputSchema), async (req, res) => {
|
||||
const id = req.params.id as string;
|
||||
const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found");
|
||||
|
|
@ -8087,6 +8192,11 @@ export function issueRoutes(
|
|||
actor,
|
||||
workProductChanged: true,
|
||||
});
|
||||
const reviewDocumentInputChanged = ["type", "provider", "metadata", "title", "createdByRunId"]
|
||||
.some((key) => Object.prototype.hasOwnProperty.call(patch, key));
|
||||
if (reviewDocumentInputChanged || sourceTrust) {
|
||||
await materializeArtifactReviewDocumentBestEffort({ issue, workProduct: product, actor });
|
||||
}
|
||||
res.json(product);
|
||||
});
|
||||
|
||||
|
|
@ -12997,7 +13107,16 @@ export function issueRoutes(
|
|||
objectContentType: object.contentType,
|
||||
originalFilename: attachment.originalFilename,
|
||||
});
|
||||
res.setHeader("Content-Type", responseContentType);
|
||||
// Markdown bodies are stored as UTF-8; declare the charset so inline
|
||||
// (raw) views do not mojibake. SVG/inline checks below stay on the bare type.
|
||||
const isMarkdownResponse = isMarkdownAttachmentContent({
|
||||
contentType: responseContentType,
|
||||
originalFilename: attachment.originalFilename,
|
||||
});
|
||||
res.setHeader(
|
||||
"Content-Type",
|
||||
isMarkdownResponse ? `${responseContentType}; charset=utf-8` : responseContentType,
|
||||
);
|
||||
res.setHeader("Cache-Control", "private, max-age=60");
|
||||
res.setHeader("X-Content-Type-Options", "nosniff");
|
||||
if (responseContentType === SVG_CONTENT_TYPE) {
|
||||
|
|
|
|||
|
|
@ -536,6 +536,14 @@ const responses = {
|
|||
description: "Conflict",
|
||||
content: { "application/json": { schema: ErrorSchema } },
|
||||
},
|
||||
payloadTooLarge: {
|
||||
description: "Payload too large",
|
||||
content: { "application/json": { schema: ErrorSchema } },
|
||||
},
|
||||
unsupportedMediaType: {
|
||||
description: "Unsupported media type",
|
||||
content: { "application/json": { schema: ErrorSchema } },
|
||||
},
|
||||
unprocessable: {
|
||||
description: "Unprocessable entity",
|
||||
content: { "application/json": { schema: ErrorSchema } },
|
||||
|
|
@ -2318,6 +2326,25 @@ registry.registerPath({
|
|||
responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized },
|
||||
});
|
||||
|
||||
registry.registerPath({
|
||||
method: "post",
|
||||
path: "/api/issues/{id}/work-products/{workProductId}/review-document",
|
||||
tags: ["issues"],
|
||||
summary: "Ensure the review document for a Markdown work product",
|
||||
request: { params: z.object({ id: z.string(), workProductId: z.string() }) },
|
||||
responses: {
|
||||
200: r.ok(),
|
||||
201: r.ok(),
|
||||
401: r.unauthorized,
|
||||
403: r.forbidden,
|
||||
404: r.notFound,
|
||||
409: r.conflict,
|
||||
413: r.payloadTooLarge,
|
||||
415: r.unsupportedMediaType,
|
||||
422: r.unprocessable,
|
||||
},
|
||||
});
|
||||
|
||||
registry.registerPath({
|
||||
method: "patch",
|
||||
path: "/api/work-products/{id}",
|
||||
|
|
|
|||
|
|
@ -0,0 +1,245 @@
|
|||
import { isDeepStrictEqual } from "node:util";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import { assets, documentRevisions, documents, issueAttachments, issueDocuments } from "@paperclipai/db";
|
||||
import {
|
||||
MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES,
|
||||
artifactReviewDocumentKey,
|
||||
getAttachmentArtifactWorkProductMetadata,
|
||||
isMarkdownAttachmentContent,
|
||||
type AttachmentArtifactWorkProductMetadata,
|
||||
type IssueWorkProduct,
|
||||
} from "@paperclipai/shared";
|
||||
import {
|
||||
HttpError,
|
||||
conflict,
|
||||
notFound,
|
||||
payloadTooLarge,
|
||||
unprocessable,
|
||||
unsupportedMediaType,
|
||||
} from "../errors.js";
|
||||
import type { StorageService } from "../storage/types.js";
|
||||
import { documentAnnotationService } from "./document-annotations.js";
|
||||
import { documentService, issueDocumentSelect, mapIssueDocumentRow } from "./documents.js";
|
||||
|
||||
export type EnsureArtifactReviewWorkProduct = Pick<
|
||||
IssueWorkProduct,
|
||||
"id" | "companyId" | "issueId" | "type" | "provider" | "metadata" | "title" | "createdByRunId" | "sourceTrust"
|
||||
>;
|
||||
|
||||
export interface EnsureArtifactReviewDocumentInput {
|
||||
issue: { id: string; companyId: string };
|
||||
workProduct: EnsureArtifactReviewWorkProduct;
|
||||
}
|
||||
|
||||
type RemappedAnnotations = Awaited<
|
||||
ReturnType<ReturnType<typeof documentAnnotationService>["remapOpenThreadsForDocument"]>
|
||||
>;
|
||||
|
||||
export interface EnsureArtifactReviewDocumentResult {
|
||||
document: ReturnType<typeof mapIssueDocumentRow> & { body?: string };
|
||||
created: boolean;
|
||||
revisionChanged: boolean;
|
||||
remappedAnnotations: RemappedAnnotations;
|
||||
}
|
||||
|
||||
/**
|
||||
* Materializes the review IssueDocument for an eligible Markdown
|
||||
* attachment-backed work product under the deterministic
|
||||
* `artifact-review-<workProductId>` key. The operation is idempotent: repeated
|
||||
* calls return the existing document and only write a new revision when the
|
||||
* review content or its work-product provenance changed.
|
||||
*/
|
||||
export function artifactReviewDocumentService(db: Db, storage: StorageService) {
|
||||
const documentsSvc = documentService(db);
|
||||
const annotationsSvc = documentAnnotationService(db);
|
||||
|
||||
const getExistingByKey = async (issueId: string, key: string) => {
|
||||
const document = await db
|
||||
.select(issueDocumentSelect)
|
||||
.from(issueDocuments)
|
||||
.innerJoin(documents, eq(issueDocuments.documentId, documents.id))
|
||||
.where(and(eq(issueDocuments.issueId, issueId), eq(issueDocuments.key, key)))
|
||||
.then((rows) => (rows[0] ? mapIssueDocumentRow(rows[0], true) : null));
|
||||
if (!document) return null;
|
||||
const latestRevision = document.latestRevisionId
|
||||
? await db
|
||||
.select({
|
||||
createdByAgentId: documentRevisions.createdByAgentId,
|
||||
createdByUserId: documentRevisions.createdByUserId,
|
||||
createdByRunId: documentRevisions.createdByRunId,
|
||||
})
|
||||
.from(documentRevisions)
|
||||
.where(and(
|
||||
eq(documentRevisions.id, document.latestRevisionId),
|
||||
eq(documentRevisions.documentId, document.id),
|
||||
eq(documentRevisions.companyId, document.companyId),
|
||||
))
|
||||
.then((rows) => rows[0] ?? null)
|
||||
: null;
|
||||
return { document, latestRevision };
|
||||
};
|
||||
|
||||
const readMarkdownAttachmentBody = async (
|
||||
issue: { id: string; companyId: string },
|
||||
metadata: AttachmentArtifactWorkProductMetadata,
|
||||
) => {
|
||||
const attachment = await db
|
||||
.select({
|
||||
id: issueAttachments.id,
|
||||
companyId: issueAttachments.companyId,
|
||||
issueId: issueAttachments.issueId,
|
||||
objectKey: assets.objectKey,
|
||||
contentType: assets.contentType,
|
||||
byteSize: assets.byteSize,
|
||||
originalFilename: assets.originalFilename,
|
||||
createdByAgentId: assets.createdByAgentId,
|
||||
createdByUserId: assets.createdByUserId,
|
||||
})
|
||||
.from(issueAttachments)
|
||||
.innerJoin(assets, eq(issueAttachments.assetId, assets.id))
|
||||
.where(eq(issueAttachments.id, metadata.attachmentId))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
// Unknown and cross-scope attachment ids are deliberately
|
||||
// indistinguishable: canonical metadata must reference an attachment on
|
||||
// the same issue and company.
|
||||
if (!attachment || attachment.companyId !== issue.companyId || attachment.issueId !== issue.id) {
|
||||
throw unprocessable("Work product attachment must reference an attachment on the same issue", {
|
||||
code: "invalid_attachment_artifact_metadata",
|
||||
attachmentId: metadata.attachmentId,
|
||||
});
|
||||
}
|
||||
if (!isMarkdownAttachmentContent(attachment)) {
|
||||
throw unsupportedMediaType("Work product attachment is not Markdown", {
|
||||
code: "unsupported_review_document_content_type",
|
||||
contentType: attachment.contentType,
|
||||
});
|
||||
}
|
||||
if (attachment.byteSize > MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES) {
|
||||
throw payloadTooLarge("Markdown attachment exceeds the review document size limit", {
|
||||
code: "review_document_too_large",
|
||||
byteSize: attachment.byteSize,
|
||||
maxBytes: MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES,
|
||||
});
|
||||
}
|
||||
|
||||
const object = await storage.getObject(attachment.companyId, attachment.objectKey);
|
||||
const chunks: Buffer[] = [];
|
||||
let total = 0;
|
||||
for await (const chunk of object.stream) {
|
||||
const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
|
||||
total += buffer.length;
|
||||
// Defense in depth: enforce the cap during streaming too, so a
|
||||
// metadata/object size mismatch can never exceed the limit.
|
||||
if (total > MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES) {
|
||||
object.stream.destroy();
|
||||
throw payloadTooLarge("Markdown attachment exceeds the review document size limit", {
|
||||
code: "review_document_too_large",
|
||||
maxBytes: MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES,
|
||||
});
|
||||
}
|
||||
chunks.push(buffer);
|
||||
}
|
||||
|
||||
let body: string;
|
||||
try {
|
||||
body = new TextDecoder("utf-8", { fatal: true }).decode(Buffer.concat(chunks));
|
||||
} catch {
|
||||
throw unprocessable("Markdown attachment is not valid UTF-8", {
|
||||
code: "invalid_review_document_encoding",
|
||||
attachmentId: attachment.id,
|
||||
});
|
||||
}
|
||||
return { attachment, body };
|
||||
};
|
||||
|
||||
return {
|
||||
ensureForWorkProduct: async (
|
||||
input: EnsureArtifactReviewDocumentInput,
|
||||
): Promise<EnsureArtifactReviewDocumentResult> => {
|
||||
const { issue, workProduct } = input;
|
||||
if (workProduct.issueId !== issue.id || workProduct.companyId !== issue.companyId) {
|
||||
throw notFound("Work product not found");
|
||||
}
|
||||
const metadata = getAttachmentArtifactWorkProductMetadata(workProduct);
|
||||
if (!metadata) {
|
||||
throw unprocessable("Work product is not an attachment-backed Paperclip artifact", {
|
||||
code: "not_attachment_backed_artifact",
|
||||
workProductId: workProduct.id,
|
||||
});
|
||||
}
|
||||
if (!isMarkdownAttachmentContent(metadata)) {
|
||||
throw unsupportedMediaType("Work product attachment is not Markdown", {
|
||||
code: "unsupported_review_document_content_type",
|
||||
contentType: metadata.contentType,
|
||||
});
|
||||
}
|
||||
|
||||
const { attachment, body } = await readMarkdownAttachmentBody(issue, metadata);
|
||||
const key = artifactReviewDocumentKey(workProduct.id);
|
||||
// The review document represents agent-authored attachment content, so
|
||||
// attribution promotes the attachment/work-product provenance instead of
|
||||
// the actor who triggered materialization.
|
||||
const attribution = {
|
||||
createdByAgentId: attachment.createdByAgentId ?? null,
|
||||
createdByUserId: attachment.createdByUserId ?? null,
|
||||
createdByRunId: workProduct.createdByRunId ?? null,
|
||||
sourceTrust: workProduct.sourceTrust ?? null,
|
||||
};
|
||||
|
||||
for (let attempt = 0; attempt < 2; attempt += 1) {
|
||||
const existingState = await getExistingByKey(issue.id, key);
|
||||
const existing = existingState?.document ?? null;
|
||||
const latestRevision = existingState?.latestRevision ?? null;
|
||||
const reviewStateMatches = existing &&
|
||||
existing.body === body &&
|
||||
existing.title === (workProduct.title ?? null) &&
|
||||
existing.format === "markdown" &&
|
||||
latestRevision?.createdByAgentId === attribution.createdByAgentId &&
|
||||
latestRevision.createdByUserId === attribution.createdByUserId &&
|
||||
latestRevision.createdByRunId === attribution.createdByRunId &&
|
||||
isDeepStrictEqual(existing.sourceTrust ?? null, attribution.sourceTrust);
|
||||
if (reviewStateMatches) {
|
||||
return { document: existing, created: false, revisionChanged: false, remappedAnnotations: [] };
|
||||
}
|
||||
try {
|
||||
const result = await documentsSvc.upsertIssueDocument({
|
||||
issueId: issue.id,
|
||||
key,
|
||||
title: workProduct.title ?? null,
|
||||
format: "markdown",
|
||||
body,
|
||||
changeSummary: existing
|
||||
? "Synced from the work product attachment"
|
||||
: "Materialized from the Markdown work product attachment",
|
||||
baseRevisionId: existing?.latestRevisionId ?? null,
|
||||
...attribution,
|
||||
lockedDocumentStrategy: "conflict",
|
||||
});
|
||||
const remappedAnnotations = result.created
|
||||
? []
|
||||
: await annotationsSvc.remapOpenThreadsForDocument({
|
||||
issueId: issue.id,
|
||||
key: result.document.key,
|
||||
documentId: result.document.id,
|
||||
nextRevisionId: result.document.latestRevisionId,
|
||||
nextRevisionNumber: result.document.latestRevisionNumber,
|
||||
nextBody: result.document.body,
|
||||
});
|
||||
return {
|
||||
document: result.document,
|
||||
created: result.created,
|
||||
revisionChanged: true,
|
||||
remappedAnnotations,
|
||||
};
|
||||
} catch (error) {
|
||||
// A concurrent materialization or document write raced this one.
|
||||
// Re-read once so identical content converges on the winner.
|
||||
if (error instanceof HttpError && error.status === 409 && attempt === 0) continue;
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
throw conflict("Concurrent review-document updates did not converge", { key });
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
@ -23,6 +23,7 @@ export {
|
|||
export { agentInstructionsService, syncInstructionsBundleConfigFromFilePath } from "./agent-instructions.js";
|
||||
export { assetService } from "./assets.js";
|
||||
export { documentService, extractLegacyPlanBody } from "./documents.js";
|
||||
export { artifactReviewDocumentService } from "./artifact-review-documents.js";
|
||||
export { statusCardService } from "./status-cards.js";
|
||||
export { finalizeStatusCardsForStalledGeneration } from "./status-card-finalization.js";
|
||||
export { documentAnnotationService } from "./document-annotations.js";
|
||||
|
|
|
|||
|
|
@ -342,6 +342,8 @@ export const issuesApi = {
|
|||
unlinkApproval: (id: string, approvalId: string) =>
|
||||
api.delete<{ ok: true }>(`/issues/${id}/approvals/${approvalId}`),
|
||||
listWorkProducts: (id: string) => api.get<IssueWorkProduct[]>(`/issues/${id}/work-products`),
|
||||
ensureWorkProductReviewDocument: (id: string, workProductId: string) =>
|
||||
api.post<IssueDocument>(`/issues/${id}/work-products/${workProductId}/review-document`, {}),
|
||||
createWorkProduct: (id: string, data: Record<string, unknown>) =>
|
||||
api.post<IssueWorkProduct>(`/issues/${id}/work-products`, data),
|
||||
updateWorkProduct: (id: string, data: Record<string, unknown>) =>
|
||||
|
|
|
|||
|
|
@ -5,7 +5,12 @@ import { pickTextColorForPillBg } from "@/lib/color-contrast";
|
|||
import { issueStatusText } from "@/lib/status-colors";
|
||||
import { copyTextToClipboard } from "@/lib/clipboard";
|
||||
import { Link } from "@/lib/router";
|
||||
import { deriveOriginatingActor, type Issue, type IssueLabel } from "@paperclipai/shared";
|
||||
import {
|
||||
deriveOriginatingActor,
|
||||
isArtifactReviewDocumentKey,
|
||||
type Issue,
|
||||
type IssueLabel,
|
||||
} from "@paperclipai/shared";
|
||||
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import { accessApi } from "../../api/access";
|
||||
import { agentsApi } from "../../api/agents";
|
||||
|
|
@ -225,10 +230,15 @@ export function IssueProperties({
|
|||
enabled: taskChatShellEnabled,
|
||||
});
|
||||
const { data: paneTabDocuments } = useIssueDocuments(taskChatShellEnabled ? issue.id : null);
|
||||
// Proxy `artifact-review-*` documents surface only through their Work
|
||||
// product row, so they must not summon the Plan or Documents surfaces.
|
||||
const paneTabStandaloneDocuments = (paneTabDocuments ?? []).filter(
|
||||
(doc) => !isArtifactReviewDocumentKey(doc.key),
|
||||
);
|
||||
const hasPlanTab =
|
||||
Boolean(paneTabPlanDocument)
|
||||
|| (paneTabAcceptedPlans?.length ?? 0) > 0
|
||||
|| (paneTabDocuments?.length ?? 0) > 0
|
||||
|| paneTabStandaloneDocuments.length > 0
|
||||
|| issue.workMode === "planning";
|
||||
// Artifacts covers the same three sources the tab body composes: work
|
||||
// products, documents (redundant with the Plan tab, intentionally), and
|
||||
|
|
@ -236,7 +246,7 @@ export function IssueProperties({
|
|||
// no longer summon the tab.
|
||||
const hasArtifactsTab =
|
||||
(paneTabWorkProducts?.length ?? 0) > 0
|
||||
|| (paneTabDocuments?.length ?? 0) > 0
|
||||
|| paneTabStandaloneDocuments.length > 0
|
||||
|| selectAgentArtifactAttachments(paneTabAttachments, paneTabWorkProducts).length > 0;
|
||||
const [paneTab, setPaneTab] = useState("properties");
|
||||
// Once a plan document exists, surface it: switch the pane to the Plan tab so
|
||||
|
|
|
|||
|
|
@ -1,10 +1,22 @@
|
|||
import { useEffect, useRef, useState } from "react";
|
||||
import type { CSSProperties } from "react";
|
||||
import { useQuery } from "@tanstack/react-query";
|
||||
import type { Issue, IssueDocument, IssueWorkProduct } from "@paperclipai/shared";
|
||||
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import type {
|
||||
AttachmentArtifactWorkProductMetadata,
|
||||
Issue,
|
||||
IssueDocument,
|
||||
IssueWorkProduct,
|
||||
} from "@paperclipai/shared";
|
||||
import {
|
||||
MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES,
|
||||
artifactReviewDocumentKey,
|
||||
getMarkdownWorkProductAttachmentMetadata,
|
||||
isArtifactReviewDocumentKey,
|
||||
} from "@paperclipai/shared";
|
||||
import {
|
||||
ChevronDown,
|
||||
ChevronRight,
|
||||
Download,
|
||||
ExternalLink,
|
||||
FileText,
|
||||
GitBranch,
|
||||
|
|
@ -15,6 +27,7 @@ import {
|
|||
Server,
|
||||
} from "lucide-react";
|
||||
import type { LucideIcon } from "lucide-react";
|
||||
import { ApiError } from "@/api/client";
|
||||
import { issuesApi } from "@/api/issues";
|
||||
import { queryKeys } from "@/lib/queryKeys";
|
||||
import { useIssueDocuments } from "@/hooks/useIssueDocuments";
|
||||
|
|
@ -122,6 +135,186 @@ function WorkProductRow({ workProduct }: { workProduct: IssueWorkProduct }) {
|
|||
return <div className={ROW_CLASS}>{body}</div>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Work-product row for an eligible Markdown artifact (LOOA-1533 gap): expands
|
||||
* in place into the shipped document review surface backed by the
|
||||
* server-materialized `artifact-review-<workProductId>` issue document instead
|
||||
* of opening the raw attachment. Raw open and download stay as explicit
|
||||
* secondary actions.
|
||||
*/
|
||||
function MarkdownWorkProductRow({
|
||||
issueId,
|
||||
workProduct,
|
||||
metadata,
|
||||
reviewDoc,
|
||||
openRequestId,
|
||||
}: {
|
||||
issueId: string;
|
||||
workProduct: IssueWorkProduct;
|
||||
metadata: AttachmentArtifactWorkProductMetadata;
|
||||
reviewDoc: IssueDocument | undefined;
|
||||
openRequestId?: number;
|
||||
}) {
|
||||
const [expanded, setExpanded] = useState(false);
|
||||
const [annotationPanelOpen, setAnnotationPanelOpen] = useState(false);
|
||||
const headerRef = useRef<HTMLDivElement | null>(null);
|
||||
const location = useLocation();
|
||||
const queryClient = useQueryClient();
|
||||
const badge = workProductStatusBadge(workProduct.status);
|
||||
const Chevron = expanded ? ChevronDown : ChevronRight;
|
||||
const tooLarge = metadata.byteSize > MARKDOWN_REVIEW_DOCUMENT_MAX_BYTES;
|
||||
|
||||
const ensure = useMutation({
|
||||
mutationFn: () => issuesApi.ensureWorkProductReviewDocument(issueId, workProduct.id),
|
||||
onSuccess: async () => {
|
||||
await queryClient.invalidateQueries({ queryKey: queryKeys.issues.documents(issueId) });
|
||||
},
|
||||
});
|
||||
|
||||
const requestPreview = () => {
|
||||
if (reviewDoc || tooLarge) return;
|
||||
if (ensure.isPending || ensure.isSuccess || ensure.isError) return;
|
||||
ensure.mutate();
|
||||
};
|
||||
|
||||
const handleToggle = () => {
|
||||
setExpanded((open) => {
|
||||
if (!open) requestPreview();
|
||||
return !open;
|
||||
});
|
||||
};
|
||||
|
||||
useEffect(() => {
|
||||
if (openRequestId === undefined) return;
|
||||
setExpanded(true);
|
||||
}, [openRequestId]);
|
||||
useEffect(() => {
|
||||
if (openRequestId === undefined || !expanded) return;
|
||||
headerRef.current?.scrollIntoView({ behavior: "smooth", block: "center" });
|
||||
}, [expanded, openRequestId]);
|
||||
// Deep links land before the user clicks, so the deep-link expansion has to
|
||||
// request materialization the same way a manual expand does.
|
||||
useEffect(() => {
|
||||
if (openRequestId === undefined) return;
|
||||
requestPreview();
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [openRequestId, reviewDoc]);
|
||||
|
||||
const unsupportedError =
|
||||
ensure.error instanceof ApiError && [413, 415, 422].includes(ensure.error.status);
|
||||
|
||||
let expandedBody: React.ReactNode;
|
||||
if (tooLarge) {
|
||||
expandedBody = (
|
||||
<p className="text-sm text-muted-foreground">
|
||||
This Markdown file is too large to preview. Use Raw or Download instead.
|
||||
</p>
|
||||
);
|
||||
} else if (reviewDoc) {
|
||||
expandedBody = reviewDoc.body.trim().length > 0 ? (
|
||||
<IssueDocumentAnnotations
|
||||
issueId={issueId}
|
||||
doc={reviewDoc}
|
||||
bodyMarkdown={reviewDoc.body}
|
||||
draftDirty={false}
|
||||
draftConflicted={false}
|
||||
historicalPreview={false}
|
||||
locationHash={location.hash}
|
||||
panelOpen={annotationPanelOpen}
|
||||
onPanelOpenChange={setAnnotationPanelOpen}
|
||||
panelPlacement="popover"
|
||||
>
|
||||
<MarkdownBody>{reviewDoc.body}</MarkdownBody>
|
||||
</IssueDocumentAnnotations>
|
||||
) : (
|
||||
<p className="text-sm text-muted-foreground">Document is empty.</p>
|
||||
);
|
||||
} else if (ensure.isError) {
|
||||
expandedBody = (
|
||||
<div className="flex flex-col items-start gap-1.5">
|
||||
<p className="text-sm text-muted-foreground">
|
||||
{unsupportedError
|
||||
? "This file can't be previewed as Markdown. Use Raw or Download instead."
|
||||
: "Preview failed to load."}
|
||||
</p>
|
||||
{!unsupportedError ? (
|
||||
<button
|
||||
type="button"
|
||||
className="rounded-md border border-border px-2 py-0.5 text-(length:--text-micro) text-muted-foreground hover:bg-accent/50"
|
||||
onClick={() => {
|
||||
ensure.reset();
|
||||
ensure.mutate();
|
||||
}}
|
||||
>
|
||||
Retry
|
||||
</button>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
} else {
|
||||
expandedBody = <p className="text-sm text-muted-foreground">Preparing preview…</p>;
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="rounded-md border border-border bg-card/50">
|
||||
<div ref={headerRef} className="flex items-center hover:bg-accent/50">
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleToggle}
|
||||
className="flex min-w-0 flex-1 items-center gap-2 px-2.5 py-1.5 text-left text-sm"
|
||||
aria-expanded={expanded}
|
||||
>
|
||||
<FileText className="h-3.5 w-3.5 shrink-0 text-muted-foreground" />
|
||||
<span className="min-w-0 flex-1 truncate">{workProduct.title}</span>
|
||||
{badge ? (
|
||||
<span
|
||||
className="status-chip inline-flex shrink-0 items-center rounded-full border px-1.5 py-0.5 text-(length:--text-nano) leading-none whitespace-nowrap"
|
||||
style={{ "--sc": `var(${badge.cssVar})` } as CSSProperties}
|
||||
>
|
||||
{badge.label}
|
||||
</span>
|
||||
) : null}
|
||||
{reviewDoc ? (
|
||||
<span className="shrink-0 text-(length:--text-micro) text-muted-foreground">
|
||||
{`Rev ${reviewDoc.latestRevisionNumber ?? 1}`}
|
||||
</span>
|
||||
) : null}
|
||||
<Chevron className="h-3.5 w-3.5 shrink-0 text-muted-foreground" />
|
||||
</button>
|
||||
{reviewDoc ? (
|
||||
<DocumentAnnotationsCountChip
|
||||
issueId={issueId}
|
||||
docKey={reviewDoc.key}
|
||||
panelOpen={annotationPanelOpen}
|
||||
onToggle={() => setAnnotationPanelOpen((open) => !open)}
|
||||
/>
|
||||
) : null}
|
||||
<a
|
||||
href={metadata.openPath}
|
||||
target="_blank"
|
||||
rel="noreferrer"
|
||||
aria-label={`Open raw ${workProduct.title}`}
|
||||
title="Open raw"
|
||||
className="shrink-0 px-1.5 py-1.5 text-muted-foreground hover:text-foreground"
|
||||
>
|
||||
<ExternalLink className="h-3 w-3" />
|
||||
</a>
|
||||
<a
|
||||
href={metadata.downloadPath}
|
||||
aria-label={`Download ${workProduct.title}`}
|
||||
title="Download"
|
||||
className="shrink-0 py-1.5 pr-2 pl-0.5 text-muted-foreground hover:text-foreground"
|
||||
>
|
||||
<Download className="h-3 w-3" />
|
||||
</a>
|
||||
</div>
|
||||
{expanded ? (
|
||||
<div className="border-t border-border px-2.5 py-2">{expandedBody}</div>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function DocumentRow({
|
||||
issueId,
|
||||
doc,
|
||||
|
|
@ -215,7 +408,10 @@ export function IssuePropertiesArtifactsTab({ issue, documentDeepLink }: IssuePr
|
|||
const { data: documents } = useIssueDocuments(issue.id);
|
||||
|
||||
const workProductRows = workProducts ?? [];
|
||||
const documentRows = documents ?? [];
|
||||
// Proxy review documents (`artifact-review-*`) present only through their
|
||||
// originating Work product row, never as standalone Documents rows.
|
||||
const documentRows = (documents ?? []).filter((doc) => !isArtifactReviewDocumentKey(doc.key));
|
||||
const reviewDocsByKey = new Map((documents ?? []).map((doc) => [doc.key, doc]));
|
||||
const fileRows = selectAgentArtifactAttachments(attachments, workProducts);
|
||||
|
||||
if (workProductRows.length === 0 && documentRows.length === 0 && fileRows.length === 0) {
|
||||
|
|
@ -232,11 +428,30 @@ export function IssuePropertiesArtifactsTab({ issue, documentDeepLink }: IssuePr
|
|||
<>
|
||||
<SectionHeading>Work products</SectionHeading>
|
||||
<ul className="flex flex-col gap-1">
|
||||
{workProductRows.map((wp) => (
|
||||
<li key={wp.id}>
|
||||
<WorkProductRow workProduct={wp} />
|
||||
</li>
|
||||
))}
|
||||
{workProductRows.map((wp) => {
|
||||
const markdownMetadata = getMarkdownWorkProductAttachmentMetadata(wp);
|
||||
if (!markdownMetadata) {
|
||||
return (
|
||||
<li key={wp.id}>
|
||||
<WorkProductRow workProduct={wp} />
|
||||
</li>
|
||||
);
|
||||
}
|
||||
const reviewKey = artifactReviewDocumentKey(wp.id);
|
||||
return (
|
||||
<li key={wp.id}>
|
||||
<MarkdownWorkProductRow
|
||||
issueId={issue.id}
|
||||
workProduct={wp}
|
||||
metadata={markdownMetadata}
|
||||
reviewDoc={reviewDocsByKey.get(reviewKey)}
|
||||
openRequestId={documentDeepLink?.documentKey === reviewKey
|
||||
? documentDeepLink.requestId
|
||||
: undefined}
|
||||
/>
|
||||
</li>
|
||||
);
|
||||
})}
|
||||
</ul>
|
||||
</>
|
||||
) : null}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,344 @@
|
|||
// @vitest-environment jsdom
|
||||
|
||||
import { flushSync } from "react-dom";
|
||||
import { createRoot, type Root } from "react-dom/client";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { Issue, IssueAttachment, IssueDocument, IssueWorkProduct } from "@paperclipai/shared";
|
||||
import { artifactReviewDocumentKey } from "@paperclipai/shared";
|
||||
import { IssuePropertiesArtifactsTab } from "./IssuePropertiesArtifactsTab";
|
||||
import { ApiError } from "@/api/client";
|
||||
|
||||
const mockIssuesApi = vi.hoisted(() => ({
|
||||
listAttachments: vi.fn(async (): Promise<unknown[]> => []),
|
||||
listWorkProducts: vi.fn(async (): Promise<unknown[]> => []),
|
||||
ensureWorkProductReviewDocument: vi.fn(async (): Promise<unknown> => ({})),
|
||||
}));
|
||||
vi.mock("@/api/issues", () => ({ issuesApi: mockIssuesApi }));
|
||||
|
||||
const mockUseIssueDocuments = vi.hoisted(() =>
|
||||
vi.fn((): { data: IssueDocument[] } => ({ data: [] })),
|
||||
);
|
||||
vi.mock("@/hooks/useIssueDocuments", () => ({ useIssueDocuments: mockUseIssueDocuments }));
|
||||
vi.mock("@/lib/router", () => ({ useLocation: () => ({ hash: "" }) }));
|
||||
vi.mock("@/components/MarkdownBody", () => ({
|
||||
MarkdownBody: ({ children }: { children: string }) => (
|
||||
<div data-testid="markdown-body">{children}</div>
|
||||
),
|
||||
}));
|
||||
vi.mock("@/components/IssueDocumentAnnotations", () => ({
|
||||
DocumentAnnotationsCountChip: ({ docKey }: { docKey: string }) => (
|
||||
<span data-testid={`annotation-count-${docKey}`} />
|
||||
),
|
||||
IssueDocumentAnnotations: ({ doc, children }: { doc: IssueDocument; children: React.ReactNode }) => (
|
||||
<div data-testid={`annotation-surface-${doc.key}`} data-document-id={doc.id}>{children}</div>
|
||||
),
|
||||
}));
|
||||
|
||||
const ATTACHMENT_ID = "00000000-0000-4000-8000-000000000001";
|
||||
const WORK_PRODUCT_ID = "11111111-1111-4111-8111-111111111111";
|
||||
const REVIEW_KEY = artifactReviewDocumentKey(WORK_PRODUCT_ID);
|
||||
|
||||
const issue = { id: "issue-1", identifier: "PAP-1534", workMode: "standard" } as Issue;
|
||||
|
||||
function makeMarkdownWorkProduct(overrides: Partial<IssueWorkProduct> = {}): IssueWorkProduct {
|
||||
const contentPath = `/api/attachments/${ATTACHMENT_ID}/content`;
|
||||
return {
|
||||
id: WORK_PRODUCT_ID,
|
||||
companyId: "company-1",
|
||||
projectId: null,
|
||||
issueId: "issue-1",
|
||||
executionWorkspaceId: null,
|
||||
runtimeServiceId: null,
|
||||
type: "artifact",
|
||||
provider: "paperclip",
|
||||
externalId: null,
|
||||
title: "Verification report",
|
||||
url: null,
|
||||
status: "ready_for_review",
|
||||
reviewState: "none",
|
||||
isPrimary: false,
|
||||
healthStatus: "unknown",
|
||||
summary: null,
|
||||
metadata: {
|
||||
attachmentId: ATTACHMENT_ID,
|
||||
contentType: "text/markdown",
|
||||
byteSize: 64,
|
||||
contentPath,
|
||||
openPath: contentPath,
|
||||
downloadPath: `${contentPath}?download=1`,
|
||||
originalFilename: "report.md",
|
||||
},
|
||||
createdByRunId: null,
|
||||
createdAt: new Date("2026-08-01T12:00:00Z"),
|
||||
updatedAt: new Date("2026-08-01T12:00:00Z"),
|
||||
...overrides,
|
||||
} as IssueWorkProduct;
|
||||
}
|
||||
|
||||
function makeReviewDocument(overrides: Partial<IssueDocument> = {}): IssueDocument {
|
||||
return {
|
||||
id: "document-1",
|
||||
companyId: "company-1",
|
||||
issueId: "issue-1",
|
||||
key: REVIEW_KEY,
|
||||
title: "Verification report",
|
||||
format: "markdown",
|
||||
body: "# Report\n\nRendered body",
|
||||
latestRevisionId: "revision-1",
|
||||
latestRevisionNumber: 1,
|
||||
createdByAgentId: null,
|
||||
createdByUserId: null,
|
||||
updatedByAgentId: null,
|
||||
updatedByUserId: null,
|
||||
lockedAt: null,
|
||||
lockedByAgentId: null,
|
||||
lockedByUserId: null,
|
||||
createdAt: new Date("2026-08-01T12:00:00Z"),
|
||||
updatedAt: new Date("2026-08-01T12:00:00Z"),
|
||||
...overrides,
|
||||
} as IssueDocument;
|
||||
}
|
||||
|
||||
function makeMarkdownAttachment(): IssueAttachment {
|
||||
return {
|
||||
id: ATTACHMENT_ID,
|
||||
companyId: "company-1",
|
||||
issueId: "issue-1",
|
||||
issueCommentId: null,
|
||||
assetId: "asset-1",
|
||||
provider: "local",
|
||||
objectKey: `objects/${ATTACHMENT_ID}`,
|
||||
contentType: "text/markdown",
|
||||
byteSize: 64,
|
||||
sha256: "0".repeat(64),
|
||||
originalFilename: "report.md",
|
||||
createdByAgentId: "agent-1",
|
||||
createdByUserId: null,
|
||||
createdAt: new Date("2026-08-01T12:00:00Z"),
|
||||
updatedAt: new Date("2026-08-01T12:00:00Z"),
|
||||
contentPath: `/api/attachments/${ATTACHMENT_ID}/content`,
|
||||
} as IssueAttachment;
|
||||
}
|
||||
|
||||
async function act(callback: () => void | Promise<void>) {
|
||||
let result: void | Promise<void> = undefined;
|
||||
flushSync(() => {
|
||||
result = callback();
|
||||
});
|
||||
await result;
|
||||
}
|
||||
|
||||
async function flush() {
|
||||
await act(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
});
|
||||
}
|
||||
|
||||
async function waitForAssertion(assertion: () => void, attempts = 20) {
|
||||
let lastError: unknown;
|
||||
for (let attempt = 0; attempt < attempts; attempt += 1) {
|
||||
try {
|
||||
assertion();
|
||||
return;
|
||||
} catch (error) {
|
||||
lastError = error;
|
||||
await flush();
|
||||
}
|
||||
}
|
||||
throw lastError;
|
||||
}
|
||||
|
||||
describe("markdown work product review row", () => {
|
||||
let container: HTMLDivElement;
|
||||
let root: Root | null = null;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
Element.prototype.scrollIntoView = vi.fn();
|
||||
mockIssuesApi.listAttachments.mockResolvedValue([]);
|
||||
mockIssuesApi.listWorkProducts.mockResolvedValue([makeMarkdownWorkProduct()]);
|
||||
mockUseIssueDocuments.mockReturnValue({ data: [] });
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
if (root) {
|
||||
const currentRoot = root;
|
||||
await act(async () => currentRoot.unmount());
|
||||
root = null;
|
||||
}
|
||||
container.remove();
|
||||
});
|
||||
|
||||
async function renderTab(props: { documentDeepLink?: { requestId: number; documentKey: string } | null } = {}) {
|
||||
const queryClient = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false }, mutations: { retry: false } },
|
||||
});
|
||||
root = createRoot(container);
|
||||
const currentRoot = root;
|
||||
await act(async () =>
|
||||
currentRoot.render(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<IssuePropertiesArtifactsTab issue={issue} {...props} />
|
||||
</QueryClientProvider>,
|
||||
),
|
||||
);
|
||||
await waitForAssertion(() => {
|
||||
expect(container.textContent).toContain("Verification report");
|
||||
});
|
||||
}
|
||||
|
||||
function expandButton() {
|
||||
return container.querySelector('button[aria-expanded]') as HTMLButtonElement;
|
||||
}
|
||||
|
||||
it("renders an expandable row with explicit raw and download actions", async () => {
|
||||
await renderTab();
|
||||
|
||||
expect(expandButton().textContent).toContain("Verification report");
|
||||
const raw = container.querySelector('a[title="Open raw"]') as HTMLAnchorElement;
|
||||
const download = container.querySelector('a[title="Download"]') as HTMLAnchorElement;
|
||||
expect(raw?.getAttribute("href")).toBe(`/api/attachments/${ATTACHMENT_ID}/content`);
|
||||
expect(raw?.getAttribute("target")).toBe("_blank");
|
||||
expect(download?.getAttribute("href")).toBe(`/api/attachments/${ATTACHMENT_ID}/content?download=1`);
|
||||
});
|
||||
|
||||
it("expands into the existing document surface without a server call when the document exists", async () => {
|
||||
mockUseIssueDocuments.mockReturnValue({ data: [makeReviewDocument()] });
|
||||
await renderTab();
|
||||
|
||||
// The proxy document maps onto the work-product row instead of a
|
||||
// standalone Documents row.
|
||||
expect(container.textContent).not.toContain("Documents");
|
||||
expect(container.querySelector(`[data-testid="annotation-count-${REVIEW_KEY}"]`)).not.toBeNull();
|
||||
|
||||
await act(async () => expandButton().click());
|
||||
expect(
|
||||
container.querySelector(`[data-testid="annotation-surface-${REVIEW_KEY}"]`)?.getAttribute("data-document-id"),
|
||||
).toBe("document-1");
|
||||
expect(container.querySelector('[data-testid="markdown-body"]')?.textContent).toContain("Rendered body");
|
||||
expect(mockIssuesApi.ensureWorkProductReviewDocument).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("materializes the document on first expand", async () => {
|
||||
mockIssuesApi.ensureWorkProductReviewDocument.mockResolvedValue(makeReviewDocument());
|
||||
await renderTab();
|
||||
|
||||
await act(async () => expandButton().click());
|
||||
expect(container.textContent).toContain("Preparing preview…");
|
||||
await waitForAssertion(() => {
|
||||
expect(mockIssuesApi.ensureWorkProductReviewDocument).toHaveBeenCalledWith("issue-1", WORK_PRODUCT_ID);
|
||||
});
|
||||
expect(mockIssuesApi.ensureWorkProductReviewDocument).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("shows an unsupported state without retry for typed rejections", async () => {
|
||||
mockIssuesApi.ensureWorkProductReviewDocument.mockRejectedValue(
|
||||
new ApiError("Work product attachment is not Markdown", 415, {}),
|
||||
);
|
||||
await renderTab();
|
||||
|
||||
await act(async () => expandButton().click());
|
||||
await waitForAssertion(() => {
|
||||
expect(container.textContent).toContain("can't be previewed as Markdown");
|
||||
});
|
||||
expect(container.textContent).not.toContain("Retry");
|
||||
});
|
||||
|
||||
it("offers an explicit retry for recoverable errors", async () => {
|
||||
mockIssuesApi.ensureWorkProductReviewDocument.mockRejectedValue(
|
||||
new ApiError("Internal error", 500, {}),
|
||||
);
|
||||
await renderTab();
|
||||
|
||||
await act(async () => expandButton().click());
|
||||
await waitForAssertion(() => {
|
||||
expect(container.textContent).toContain("Preview failed to load.");
|
||||
});
|
||||
|
||||
const retry = Array.from(container.querySelectorAll("button")).find(
|
||||
(button) => button.textContent === "Retry",
|
||||
);
|
||||
expect(retry).toBeDefined();
|
||||
await act(async () => retry?.click());
|
||||
await waitForAssertion(() => {
|
||||
expect(mockIssuesApi.ensureWorkProductReviewDocument).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
it("marks oversized markdown as too large without calling the server", async () => {
|
||||
mockIssuesApi.listWorkProducts.mockResolvedValue([
|
||||
makeMarkdownWorkProduct({
|
||||
metadata: {
|
||||
...(makeMarkdownWorkProduct().metadata as Record<string, unknown>),
|
||||
byteSize: 512 * 1024 + 1,
|
||||
},
|
||||
}),
|
||||
]);
|
||||
await renderTab();
|
||||
|
||||
await act(async () => expandButton().click());
|
||||
expect(container.textContent).toContain("too large to preview");
|
||||
expect(mockIssuesApi.ensureWorkProductReviewDocument).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("expands from a document deep link and requests materialization", async () => {
|
||||
mockIssuesApi.ensureWorkProductReviewDocument.mockResolvedValue(makeReviewDocument());
|
||||
await renderTab({ documentDeepLink: { documentKey: REVIEW_KEY, requestId: 1 } });
|
||||
|
||||
await waitForAssertion(() => {
|
||||
expect(container.querySelector('button[aria-expanded="true"]')).not.toBeNull();
|
||||
expect(mockIssuesApi.ensureWorkProductReviewDocument).toHaveBeenCalledTimes(1);
|
||||
expect(Element.prototype.scrollIntoView).toHaveBeenCalledWith({ behavior: "smooth", block: "center" });
|
||||
});
|
||||
});
|
||||
|
||||
it("hides the promoted markdown attachment from Files but keeps loose attachments", async () => {
|
||||
const loose = {
|
||||
...makeMarkdownAttachment(),
|
||||
id: "22222222-2222-4222-8222-222222222222",
|
||||
originalFilename: "loose-notes.md",
|
||||
contentPath: "/api/attachments/22222222-2222-4222-8222-222222222222/content",
|
||||
};
|
||||
mockIssuesApi.listAttachments.mockResolvedValue([makeMarkdownAttachment(), loose]);
|
||||
await renderTab();
|
||||
|
||||
await waitForAssertion(() => {
|
||||
expect(container.textContent).toContain("loose-notes.md");
|
||||
});
|
||||
expect(container.textContent).not.toContain("report.md");
|
||||
const looseLink = Array.from(container.querySelectorAll("a")).find(
|
||||
(anchor) => anchor.textContent?.includes("loose-notes.md"),
|
||||
);
|
||||
expect(looseLink?.getAttribute("href")).toBe("/api/attachments/22222222-2222-4222-8222-222222222222/content");
|
||||
});
|
||||
|
||||
it("keeps non-markdown work products on the raw link row", async () => {
|
||||
const contentPath = `/api/attachments/${ATTACHMENT_ID}/content`;
|
||||
mockIssuesApi.listWorkProducts.mockResolvedValue([
|
||||
makeMarkdownWorkProduct({
|
||||
metadata: {
|
||||
attachmentId: ATTACHMENT_ID,
|
||||
contentType: "application/pdf",
|
||||
byteSize: 64,
|
||||
contentPath,
|
||||
openPath: contentPath,
|
||||
downloadPath: `${contentPath}?download=1`,
|
||||
originalFilename: "report.pdf",
|
||||
},
|
||||
}),
|
||||
]);
|
||||
await renderTab();
|
||||
|
||||
await waitForAssertion(() => {
|
||||
const row = Array.from(container.querySelectorAll("a")).find(
|
||||
(anchor) => anchor.textContent?.includes("Verification report"),
|
||||
);
|
||||
expect(row?.getAttribute("href")).toBe(contentPath);
|
||||
expect(row?.getAttribute("target")).toBe("_blank");
|
||||
});
|
||||
expect(container.querySelector("button[aria-expanded]")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,6 +1,7 @@
|
|||
import { useState } from "react";
|
||||
import { useQuery } from "@tanstack/react-query";
|
||||
import type { Issue, IssueDocument, IssueThreadInteraction } from "@paperclipai/shared";
|
||||
import { isArtifactReviewDocumentKey } from "@paperclipai/shared";
|
||||
import { issuesApi } from "@/api/issues";
|
||||
import { queryKeys } from "@/lib/queryKeys";
|
||||
import { IssuePlanDecompositionsSection } from "@/components/IssuePlanDecompositionsSection";
|
||||
|
|
@ -92,8 +93,11 @@ export function IssuePropertiesPlansTab({ issue }: IssuePropertiesPlansTabProps)
|
|||
const hasPlans = (data?.length ?? 0) > 0;
|
||||
const pendingPlanConfirmation = hasPendingPlanConfirmation(interactions);
|
||||
// Every other non-system document (e.g. `synthesis`) renders below the plan;
|
||||
// the `plan` doc itself stays on its dedicated annotated surface above.
|
||||
const otherDocuments = (documents ?? []).filter((doc) => doc.key !== "plan");
|
||||
// the `plan` doc itself stays on its dedicated annotated surface above, and
|
||||
// proxy `artifact-review-*` documents surface only through their Work product.
|
||||
const otherDocuments = (documents ?? []).filter(
|
||||
(doc) => doc.key !== "plan" && !isArtifactReviewDocumentKey(doc.key),
|
||||
);
|
||||
|
||||
if (!planDocument && !hasPlans && otherDocuments.length === 0) {
|
||||
return (
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
|
|||
import type { IssueAttachment, IssueWorkProduct } from "@paperclipai/shared";
|
||||
import {
|
||||
documentDisplayTitle,
|
||||
getAttachmentBackedWorkProductAttachmentIds,
|
||||
isAgentAttachment,
|
||||
selectAgentArtifactAttachments,
|
||||
workProductHref,
|
||||
|
|
@ -112,6 +113,54 @@ describe("selectAgentArtifactAttachments", () => {
|
|||
it("tolerates missing inputs", () => {
|
||||
expect(selectAgentArtifactAttachments(null, null)).toEqual([]);
|
||||
});
|
||||
|
||||
it("dedupes markdown attachments promoted to work products", () => {
|
||||
// Markdown is excluded from the binary Output surface, so this dedupe must
|
||||
// not depend on getPromotedOutputAttachmentIds (LOOA-1533 duplicate rows).
|
||||
const promotedId = "00000000-0000-4000-8000-000000000002";
|
||||
const promoted = makeAttachment({
|
||||
id: promotedId,
|
||||
createdByAgentId: "agent-1",
|
||||
contentType: "text/markdown",
|
||||
originalFilename: "report.md",
|
||||
});
|
||||
const workProduct = makePromotingWorkProduct(promotedId);
|
||||
workProduct.metadata = {
|
||||
...(workProduct.metadata as Record<string, unknown>),
|
||||
contentType: "text/markdown",
|
||||
originalFilename: "report.md",
|
||||
};
|
||||
expect(selectAgentArtifactAttachments([promoted], [workProduct])).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("getAttachmentBackedWorkProductAttachmentIds", () => {
|
||||
it("collects attachment ids across content types", () => {
|
||||
const imageId = "00000000-0000-4000-8000-000000000001";
|
||||
const markdownId = "00000000-0000-4000-8000-000000000002";
|
||||
const markdownWorkProduct = makePromotingWorkProduct(markdownId);
|
||||
markdownWorkProduct.metadata = {
|
||||
...(markdownWorkProduct.metadata as Record<string, unknown>),
|
||||
contentType: "text/markdown",
|
||||
originalFilename: "report.md",
|
||||
};
|
||||
const ids = getAttachmentBackedWorkProductAttachmentIds([
|
||||
makePromotingWorkProduct(imageId),
|
||||
markdownWorkProduct,
|
||||
]);
|
||||
expect(ids).toEqual(new Set([imageId, markdownId]));
|
||||
});
|
||||
|
||||
it("ignores non-canonical or non-paperclip work products", () => {
|
||||
const attachmentId = "00000000-0000-4000-8000-000000000003";
|
||||
const foreign = { ...makePromotingWorkProduct(attachmentId), provider: "github" };
|
||||
const invalid = {
|
||||
...makePromotingWorkProduct(attachmentId),
|
||||
metadata: { attachmentId, openPath: "https://evil.example/content" },
|
||||
};
|
||||
expect(getAttachmentBackedWorkProductAttachmentIds([foreign, invalid])).toEqual(new Set());
|
||||
expect(getAttachmentBackedWorkProductAttachmentIds(null)).toEqual(new Set());
|
||||
});
|
||||
});
|
||||
|
||||
describe("workProductHref", () => {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import type { IssueAttachment, IssueDocumentSummary, IssueWorkProduct } from "@paperclipai/shared";
|
||||
import { getPromotedOutputAttachmentIds } from "./issue-output";
|
||||
import { getAttachmentArtifactWorkProductMetadata } from "@paperclipai/shared";
|
||||
|
||||
/**
|
||||
* Selectors for the properties pane's Artifacts tab (PAP-491): which
|
||||
|
|
@ -19,6 +19,23 @@ export function isAgentAttachment(
|
|||
return !attachment.createdByUserId && !attachment.issueCommentId;
|
||||
}
|
||||
|
||||
/**
|
||||
* Attachment ids owned by valid attachment-backed Paperclip work products.
|
||||
* Unlike `getPromotedOutputAttachmentIds`, this includes document-like content
|
||||
* types (e.g. Markdown) that the binary Output surface intentionally excludes,
|
||||
* so it is the right dedupe set for the Artifacts tab's Files section.
|
||||
*/
|
||||
export function getAttachmentBackedWorkProductAttachmentIds(
|
||||
workProducts: IssueWorkProduct[] | null | undefined,
|
||||
): Set<string> {
|
||||
const ids = new Set<string>();
|
||||
for (const workProduct of workProducts ?? []) {
|
||||
const metadata = getAttachmentArtifactWorkProductMetadata(workProduct);
|
||||
if (metadata) ids.add(metadata.attachmentId);
|
||||
}
|
||||
return ids;
|
||||
}
|
||||
|
||||
/**
|
||||
* Agent-authored attachments minus the ones already promoted to
|
||||
* attachment-backed work products (`metadata.attachmentId`), so the Artifacts
|
||||
|
|
@ -28,7 +45,7 @@ export function selectAgentArtifactAttachments(
|
|||
attachments: IssueAttachment[] | null | undefined,
|
||||
workProducts: IssueWorkProduct[] | null | undefined,
|
||||
): IssueAttachment[] {
|
||||
const promoted = getPromotedOutputAttachmentIds(workProducts);
|
||||
const promoted = getAttachmentBackedWorkProductAttachmentIds(workProducts);
|
||||
return (attachments ?? []).filter(
|
||||
(attachment) => isAgentAttachment(attachment) && !promoted.has(attachment.id),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -1,12 +1,7 @@
|
|||
import type { IssueAttachment } from "@paperclipai/shared";
|
||||
import { isMarkdownAttachmentContent } from "@paperclipai/shared";
|
||||
import { isVideoLikeOutput } from "./issue-output";
|
||||
|
||||
const GENERIC_ATTACHMENT_CONTENT_TYPES = new Set([
|
||||
"application/octet-stream",
|
||||
"binary/octet-stream",
|
||||
"application/x-binary",
|
||||
]);
|
||||
|
||||
type AttachmentPathLike = {
|
||||
contentPath: string;
|
||||
openPath?: string;
|
||||
|
|
@ -42,17 +37,5 @@ export function isVideoAttachment(
|
|||
export function isMarkdownAttachment(
|
||||
attachment: Pick<IssueAttachment, "contentType" | "originalFilename">,
|
||||
) {
|
||||
const contentType = normalizedContentType(attachment);
|
||||
if (
|
||||
contentType === "text/markdown" ||
|
||||
contentType === "text/x-markdown" ||
|
||||
contentType === "application/markdown" ||
|
||||
contentType === "application/x-markdown"
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const filename = (attachment.originalFilename ?? "").toLowerCase();
|
||||
if (!filename.endsWith(".md") && !filename.endsWith(".markdown")) return false;
|
||||
return contentType === "text/plain" || GENERIC_ATTACHMENT_CONTENT_TYPES.has(contentType);
|
||||
return isMarkdownAttachmentContent(attachment);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue