From 3bbf7aeb7bcd420316e491501a585862378a68c4 Mon Sep 17 00:00:00 2001 From: Andrew Louis Date: Mon, 28 Sep 2026 20:24:17 -0700 Subject: [PATCH] fix(session): keep a session connected when a reload leaves out a noted file A `hunk session reload` whose new diff leaves out a file that holds a live note disconnected the session. The window's next snapshot carried `liveCommentCount: 1` beside an empty `liveComments` list: the count came from every live note in the review store, and the list came from the files in the current diff. The daemon's snapshot parser requires the two to match, so it rejected the snapshot and dropped the session. The TUI kept running, and no `hunk session` command could reach it. The count now comes from the published summaries, as `reviewNoteCount` already does. The note stays in the store and returns to both when a later reload brings its file back. --- .changeset/reload-retired-note-count.md | 5 ++ .../src/ui/hooks/useTerminalReview.test.tsx | 48 ++++++++++++ .../hunk/src/ui/hooks/useTerminalReview.ts | 6 +- test/session/cli.test.ts | 75 +++++++++++++++++++ 4 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 .changeset/reload-retired-note-count.md diff --git a/.changeset/reload-retired-note-count.md b/.changeset/reload-retired-note-count.md new file mode 100644 index 000000000..bd0ad5bee --- /dev/null +++ b/.changeset/reload-retired-note-count.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +`hunk session reload` onto a diff that leaves out a file holding a live note keeps the session connected, and the note returns when a later reload brings its file back. diff --git a/packages/hunk/src/ui/hooks/useTerminalReview.test.tsx b/packages/hunk/src/ui/hooks/useTerminalReview.test.tsx index e347798a2..c407de2c0 100644 --- a/packages/hunk/src/ui/hooks/useTerminalReview.test.tsx +++ b/packages/hunk/src/ui/hooks/useTerminalReview.test.tsx @@ -684,6 +684,54 @@ describe("useTerminalReview", () => { } }); + test("a reload that retires a noted file publishes a count that matches its summaries", async () => { + // The daemon refuses a snapshot whose live comment count differs from its list, so a note + // kept for a retired file must leave both until a later reload brings the file back. + const createAlpha = () => + createDiffFile("alpha", "alpha.ts", "export const alpha = 1;\n", "export const alpha = 2;\n"); + const createBeta = () => + createDiffFile("beta", "beta.ts", "export const beta = 1;\n", "export const beta = 2;\n"); + const { controllerRef, setFilesRef, setup } = await renderTerminalReview([ + createAlpha(), + createBeta(), + ]); + + try { + await flush(setup); + await act(async () => { + expectValue(controllerRef.current).addLiveComment( + { filePath: "beta.ts", side: "new", line: 1, summary: "Check beta rename" }, + "comment-1", + { reveal: false }, + ); + }); + await flush(setup); + expect(expectValue(controllerRef.current).liveCommentCount).toBe(1); + + await act(async () => { + expectValue(setFilesRef.current)([createAlpha()]); + }); + await flush(setup); + + expect(expectValue(controllerRef.current).liveCommentSummaries).toEqual([]); + expect(expectValue(controllerRef.current).liveCommentCount).toBe(0); + + await act(async () => { + expectValue(setFilesRef.current)([createAlpha(), createBeta()]); + }); + await flush(setup); + + expect( + expectValue(controllerRef.current).liveCommentSummaries.map((comment) => comment.commentId), + ).toEqual(["comment-1"]); + expect(expectValue(controllerRef.current).liveCommentCount).toBe(1); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("live comments validate markup at the published live width", async () => { const noteGeometry: { current: { layout: "split" | "unified"; width: number } | null } = { current: { layout: "unified", width: 120 }, diff --git a/packages/hunk/src/ui/hooks/useTerminalReview.ts b/packages/hunk/src/ui/hooks/useTerminalReview.ts index 1f2e1cc6d..0d0aca9f4 100644 --- a/packages/hunk/src/ui/hooks/useTerminalReview.ts +++ b/packages/hunk/src/ui/hooks/useTerminalReview.ts @@ -1865,8 +1865,10 @@ export function useTerminalReview({ draftNote, expandedGapsByFileId, filter, - // Counted from the store, so notes on a file a reload retired still count as tracked. - liveCommentCount: state.liveNotes.length, + // Counted from the published summaries: the daemon refuses a snapshot whose count differs + // from its list. A note on a file a reload retired stays in the store and returns to both + // when a later reload brings the file back. + liveCommentCount: liveCommentSummaries.length, liveCommentSummaries, liveCommentsByFileId, lineCursor, diff --git a/test/session/cli.test.ts b/test/session/cli.test.ts index ee7efff5b..a249d79d2 100644 --- a/test/session/cli.test.ts +++ b/test/session/cli.test.ts @@ -465,6 +465,81 @@ sessionDescribe("session CLI integration", () => { } }, 20_000); + test("reload away from a noted file keeps the session and brings the note back with the file", async () => { + const port = await reserveLoopbackPort(); + const fixture = createFixtureFiles( + "reload-retire", + ["export const alpha = 1;"], + ["export const alpha = 2;"], + ); + mkdirSync(join(fixture.dir, ".git")); + const otherBefore = join(fixture.dir, "reload-other-before.ts"); + const otherAfter = join(fixture.dir, "reload-other-after.ts"); + writeFileSync(otherBefore, "export const other = 1;\n"); + writeFileSync(otherAfter, "export const other = 2;\n"); + const session = spawnHunkSession(fixture, port); + + try { + const listed = await waitForRegisteredSessions(port); + const sessionId = listed[0]!.sessionId; + + const comment = runSessionCli( + [ + "comment", + "add", + sessionId, + "--file", + fixture.afterName, + "--new-line", + "1", + "--summary", + "Alpha note", + "--json", + ], + port, + ); + expect(comment.proc.exitCode).toBe(0); + expect(comment.stderr).toBe(""); + + // The new diff leaves out the noted file, so the note stays in the window's store only. + const away = runSessionCli( + ["reload", sessionId, "--json", "--", "diff", "--files", otherBefore, otherAfter], + port, + ); + expect(away.stderr).toBe(""); + expect(away.proc.exitCode).toBe(0); + + const listedAway = runSessionCli(["list", "--json"], port); + expect(listedAway.proc.exitCode).toBe(0); + expect( + (JSON.parse(listedAway.stdout) as SessionListJson).sessions.map((entry) => entry.sessionId), + ).toEqual([sessionId]); + + const back = runSessionCli( + ["reload", sessionId, "--json", "--", "diff", "--files", fixture.before, fixture.after], + port, + ); + expect(back.stderr).toBe(""); + expect(back.proc.exitCode).toBe(0); + + const returned = await waitUntil("note back on its file", () => { + const listedComments = runSessionCli(["comment", "list", sessionId, "--json"], port); + if (listedComments.proc.exitCode !== 0) { + return null; + } + const parsed = JSON.parse(listedComments.stdout) as { + comments?: Array<{ summary?: string; filePath?: string }>; + }; + return parsed.comments?.some((entry) => entry.summary === "Alpha note") ? parsed : null; + }); + expect(returned.comments).toMatchObject([ + { summary: "Alpha note", filePath: fixture.afterName }, + ]); + } finally { + await cleanupHunkSession(session, fixture, port); + } + }, 20_000); + test("reload refuses to read files outside the live session root", async () => { const port = await reserveLoopbackPort(); const fixture = createFixtureFiles(