Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/content-anchored-comments.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"diffhub": patch
---

Add content-anchored resolvable comments with navigation, reply threads, and staleness detection
134 changes: 133 additions & 1 deletion apps/cli/app/api/comments/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { mkdirSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { DELETE, GET, POST } from "./route";
import { DELETE, GET, PATCH, POST } from "./route";

const tempPaths: string[] = [];

Expand All @@ -19,6 +19,7 @@ describe("/api/comments", () => {
});

afterEach(() => {
delete process.env.DIFFHUB_USER;
delete process.env.DIFFHUB_REPO;

for (const tempPath of tempPaths.splice(0)) {
Expand Down Expand Up @@ -88,6 +89,29 @@ describe("/api/comments", () => {
});
});

it("rejects non-positive and non-integer line numbers", async () => {
for (const lineNumber of [0, -1, 1.5]) {
const response = await POST(
new Request("http://localhost/api/comments", {
body: JSON.stringify({
body: "Review this hunk",
file: "src/a.ts",
lineNumber,
side: "right",
tag: "",
}),
headers: { "Content-Type": "application/json" },
method: "POST",
}),
);

expect(response.status).toBe(400);
await expect(response.json()).resolves.toStrictEqual({
error: "lineNumber must be a positive integer",
});
}
});

it("requires an id to delete a comment", async () => {
const response = await DELETE(
new Request("http://localhost/api/comments", { method: "DELETE" }),
Expand Down Expand Up @@ -125,4 +149,112 @@ describe("/api/comments", () => {
await expect(response.json()).resolves.toStrictEqual({ ok: true });
await expect(GET().json()).resolves.toStrictEqual([]);
});

it("resolves, replies, and unresolves comments through PATCH", async () => {
process.env.DIFFHUB_USER = "Route Tester";
const createResponse = await POST(
new Request("http://localhost/api/comments", {
body: JSON.stringify({
body: "Review this hunk",
file: "src/a.ts",
lineNumber: 12,
side: "right",
tag: "",
}),
headers: { "Content-Type": "application/json" },
method: "POST",
}),
);
const created = (await createResponse.json()) as { id: string };

const resolveResponse = await PATCH(
new Request(`http://localhost/api/comments?id=${created.id}`, {
body: JSON.stringify({ action: "resolve" }),
headers: { "Content-Type": "application/json" },
method: "PATCH",
}),
);
expect(resolveResponse.status).toBe(200);
await expect(resolveResponse.json()).resolves.toMatchObject({
resolved: true,
resolvedAt: expect.any(String),
resolvedBy: "Route Tester",
});

const replyResponse = await PATCH(
new Request(`http://localhost/api/comments?id=${created.id}`, {
body: JSON.stringify({ action: "reply", body: "Fixed now" }),
headers: { "Content-Type": "application/json" },
method: "PATCH",
}),
);
expect(replyResponse.status).toBe(200);
await expect(replyResponse.json()).resolves.toMatchObject({
replies: [{ at: expect.any(String), body: "Fixed now", by: "Route Tester" }],
resolved: true,
});

const unresolveResponse = await PATCH(
new Request(`http://localhost/api/comments?id=${created.id}`, {
body: JSON.stringify({ action: "unresolve" }),
headers: { "Content-Type": "application/json" },
method: "PATCH",
}),
);
expect(unresolveResponse.status).toBe(200);
const unresolved = (await unresolveResponse.json()) as {
resolved: boolean;
resolvedAt?: string;
resolvedBy?: string;
};
expect(unresolved.resolved).toBeFalsy();
expect(unresolved.resolvedAt).toBeUndefined();
expect(unresolved.resolvedBy).toBeUndefined();
});

it("validates PATCH requests", async () => {
const missingIdResponse = await PATCH(
new Request("http://localhost/api/comments", {
body: JSON.stringify({ action: "resolve" }),
headers: { "Content-Type": "application/json" },
method: "PATCH",
}),
);
expect(missingIdResponse.status).toBe(400);
await expect(missingIdResponse.json()).resolves.toStrictEqual({ error: "id required" });

const unknownCommentResponse = await PATCH(
new Request("http://localhost/api/comments?id=missing-id", {
body: JSON.stringify({ action: "resolve" }),
headers: { "Content-Type": "application/json" },
method: "PATCH",
}),
);
expect(unknownCommentResponse.status).toBe(404);
await expect(unknownCommentResponse.json()).resolves.toStrictEqual({
error: "comment not found",
});

const invalidActionResponse = await PATCH(
new Request("http://localhost/api/comments?id=missing-id", {
body: JSON.stringify({ action: "invalid" }),
headers: { "Content-Type": "application/json" },
method: "PATCH",
}),
);
expect(invalidActionResponse.status).toBe(400);
await expect(invalidActionResponse.json()).resolves.toStrictEqual({
error: "invalid action",
});

const emptyReplyResponse = await PATCH(
new Request("http://localhost/api/comments?id=missing-id", {
body: JSON.stringify({ action: "reply", body: " " }),
headers: { "Content-Type": "application/json" },
method: "PATCH",
}),
);
expect(emptyReplyResponse.status).toBe(400);
await expect(emptyReplyResponse.json()).resolves.toStrictEqual({ error: "body is required" });
});
});
59 changes: 56 additions & 3 deletions apps/cli/app/api/comments/route.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import { NextResponse } from "next/server";
import { parseCommentSide } from "@/lib/comment-sides";
import { addComment, clearComments, deleteComment, readComments } from "@/lib/comments";
import {
addComment,
addReply,
clearComments,
deleteComment,
readComments,
setCommentResolved,
} from "@/lib/comments";
import type { CommentTag } from "@/lib/comments";

const VALID_TAGS = new Set<CommentTag>(["[must-fix]", "[suggestion]", "[nit]", "[question]", ""]);
Expand All @@ -22,8 +29,13 @@ export const POST = async (request: Request) => {
if (typeof data.file !== "string" || !data.file) {
return NextResponse.json({ error: "file is required" }, { status: 400 });
}
if (typeof data.lineNumber !== "number" || !Number.isFinite(data.lineNumber)) {
return NextResponse.json({ error: "lineNumber must be a number" }, { status: 400 });
if (
typeof data.lineNumber !== "number" ||
!Number.isFinite(data.lineNumber) ||
!Number.isInteger(data.lineNumber) ||
data.lineNumber < 1
) {
return NextResponse.json({ error: "lineNumber must be a positive integer" }, { status: 400 });
}
if (typeof data.body !== "string" || !data.body) {
return NextResponse.json({ error: "body is required" }, { status: 400 });
Expand All @@ -45,6 +57,8 @@ export const POST = async (request: Request) => {
try {
const comment = await addComment({
body: data.body,
createdBy: typeof data.createdBy === "string" ? data.createdBy : undefined,
diffHunkHeader: typeof data.diffHunkHeader === "string" ? data.diffHunkHeader : undefined,
file: data.file,
lineNumber: data.lineNumber,
side,
Expand All @@ -59,6 +73,45 @@ export const POST = async (request: Request) => {
}
};

export const PATCH = async (request: Request) => {
const { searchParams } = new URL(request.url);
const id = searchParams.get("id");
if (!id) {
return NextResponse.json({ error: "id required" }, { status: 400 });
}

const data = (await request.json()) as Record<string, unknown>;
const { action } = data;

try {
if (action === "resolve" || action === "unresolve") {
const comment = await setCommentResolved(id, action === "resolve");
if (!comment) {
return NextResponse.json({ error: "comment not found" }, { status: 404 });
}
return NextResponse.json(comment);
}

if (action === "reply") {
if (typeof data.body !== "string" || !data.body.trim()) {
return NextResponse.json({ error: "body is required" }, { status: 400 });
}
const comment = await addReply(id, data.body.trim());
if (!comment) {
return NextResponse.json({ error: "comment not found" }, { status: 404 });
}
return NextResponse.json(comment);
}

return NextResponse.json({ error: "invalid action" }, { status: 400 });
} catch (error) {
return NextResponse.json(
{ error: error instanceof Error ? error.message : "Internal server error" },
{ status: 500 },
);
}
};

export const DELETE = async (request: Request) => {
const { searchParams } = new URL(request.url);
const clearAll = searchParams.get("all") === "1";
Expand Down
42 changes: 31 additions & 11 deletions apps/cli/components/DiffApp.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { DiffApp } from "./DiffApp";
import type { Comment } from "@/lib/comment-types";
import { WATCH_STREAM_EVENTS } from "@/lib/watch-stream";

interface MockAnnotation {
lineNumber: number;
metadata?: unknown;
Expand Down Expand Up @@ -101,6 +100,21 @@ const filesPayload = {
insertions: 2,
};

const withCommentDefaults = (
comment: Omit<Comment, "anchor" | "replies" | "resolved" | "staleness">,
): Comment => ({
...comment,
anchor: {
afterContext: [],
beforeContext: [],
fileSha: "",
lineContent: "",
},
replies: [],
resolved: false,
staleness: "fresh",
});

const diffPayload = {
baseBranch: "origin/main",
branch: "feature/diff-review",
Expand Down Expand Up @@ -205,12 +219,15 @@ describe("DiffApp review flow", () => {
}

if (url === "/api/comments" && method === "POST") {
const body = JSON.parse(String(init?.body ?? "{}")) as Omit<Comment, "createdAt" | "id">;
const createdComment: Comment = {
const body = JSON.parse(String(init?.body ?? "{}")) as Omit<
Comment,
"anchor" | "createdAt" | "id" | "replies" | "resolved" | "staleness"
>;
const createdComment = withCommentDefaults({
...body,
createdAt: "2026-04-15T00:00:00.000Z",
id: `comment-${comments.length + 1}`,
};
});
comments = [...comments, createdComment];
return Promise.resolve(jsonResponse(createdComment));
}
Expand Down Expand Up @@ -301,15 +318,15 @@ describe("DiffApp review flow", () => {
value: { writeText },
});
let comments: Comment[] = [
{
withCommentDefaults({
body: "Investigate this diff",
createdAt: "2026-04-15T00:00:00.000Z",
file: "src/b.ts",
id: "comment-1",
lineNumber: 12,
side: "right",
tag: "[must-fix]",
},
}),
];

const fetchMock = vi.fn<typeof fetch>((input, init) => {
Expand Down Expand Up @@ -511,7 +528,7 @@ describe("DiffApp review flow", () => {
await screen.findByText(/watched/);
await screen.findByText("Updated just now");
expect(countFetchCalls(fetchMock, "/api/files")).toBeGreaterThanOrEqual(2);
expect(countFetchCalls(fetchMock, "/api/comments")).toBe(1);
expect(countFetchCalls(fetchMock, "/api/comments")).toBeGreaterThanOrEqual(2);
expect(countFetchCalls(fetchMock, "/api/diff")).toBeGreaterThanOrEqual(2);
expect(FakeEventSource.instances[0]?.url).toBe("/api/watch");

Expand Down Expand Up @@ -584,7 +601,7 @@ describe("DiffApp review flow", () => {
expect(
fetchMock.mock.calls.some(([input]) => String(input).startsWith("/api/files?refresh=1")),
).toBeTruthy();
expect(countFetchCalls(fetchMock, "/api/comments")).toBe(1);
expect(countFetchCalls(fetchMock, "/api/comments")).toBeGreaterThanOrEqual(2);

unmount();
});
Expand Down Expand Up @@ -658,12 +675,15 @@ describe("DiffApp review flow", () => {
}

if (url === "/api/comments" && method === "POST") {
const body = JSON.parse(String(init?.body ?? "{}")) as Omit<Comment, "createdAt" | "id">;
const createdComment: Comment = {
const body = JSON.parse(String(init?.body ?? "{}")) as Omit<
Comment,
"anchor" | "createdAt" | "id" | "replies" | "resolved" | "staleness"
>;
const createdComment = withCommentDefaults({
...body,
createdAt: "2026-04-15T00:00:00.000Z",
id: "comment-1",
};
});
comments = [createdComment];
return Promise.resolve(jsonResponse(createdComment));
}
Expand Down
Loading
Loading