diff --git a/README.md b/README.md index 123b3bf..c28767c 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,7 @@ Use it to: - inspect the current review context - jump to a file, hunk, or line - reload the current window with a different `diff` or `show` command -- add, list, and remove inline comments +- add, batch-apply, list, and remove inline comments (by hunk or by line) Most users only need `hunk session ...`. Use `hunk mcp serve` only for manual startup or debugging of the local daemon. @@ -175,13 +175,18 @@ hunk session reload --repo . -- diff hunk session reload --repo /path/to/worktree -- diff hunk session reload --session-path /path/to/live-window --source /path/to/other-checkout -- diff hunk session reload --repo . -- show HEAD~1 -- README.md +hunk session comment add --repo . --file README.md --hunk 2 --summary "Explain this hunk" hunk session comment add --repo . --file README.md --new-line 103 --summary "Tighten this wording" +hunk session comment add --repo . --file README.md --new-line 103 --summary "Tighten this wording" --focus +printf '%s\n' '{"comments":[{"filePath":"README.md","hunk":2,"summary":"Explain this hunk"}]}' | hunk session comment apply --repo . --stdin +printf '%s\n' '{"comments":[{"filePath":"README.md","hunk":2,"summary":"Explain this hunk"}]}' | hunk session comment apply --repo . --stdin --focus hunk session comment list --repo . hunk session comment rm --repo . hunk session comment clear --repo . --file README.md --yes ``` `hunk session reload ... -- ` swaps what a live session is showing without opening a new TUI window. +Pass `--focus` to jump the live session to a new note or the first note in a batch apply. - `--repo ` selects the live session by its current loaded repo root. - `--source ` is reload-only: it changes where the nested `diff` / `show` command runs, but does not select the session. diff --git a/skills/hunk-review/SKILL.md b/skills/hunk-review/SKILL.md index 907a3c7..cfc9126 100644 --- a/skills/hunk-review/SKILL.md +++ b/skills/hunk-review/SKILL.md @@ -17,7 +17,8 @@ If no session exists, ask the user to launch Hunk in their terminal first. 3. hunk session context --repo . # check current focus 4. hunk session navigate ... # move to the right place 5. hunk session reload -- # swap contents if needed -6. hunk session comment add ... # leave review notes +6. hunk session comment add ... # leave one review note +7. hunk session comment apply ... # apply many agent notes in one stdin batch ``` ## Session selection @@ -91,14 +92,20 @@ hunk session reload --session-path /path/to/live-window --source /path/to/other- ### Comments ```bash -hunk session comment add --repo . --file README.md --new-line 103 --summary "Tighten this wording" [--rationale "..."] [--author "agent"] [--no-reveal] +hunk session comment add --repo . --file README.md --hunk 2 --summary "Explain the hunk" [--rationale "..."] [--author "agent"] [--focus] +hunk session comment add --repo . --file README.md --new-line 103 --summary "Tighten this wording" [--rationale "..."] [--author "agent"] [--focus] +printf '%s\n' '{"comments":[{"filePath":"README.md","hunk":2,"summary":"Explain the hunk"}]}' | hunk session comment apply --repo . --stdin [--focus] hunk session comment list --repo . [--file README.md] hunk session comment rm --repo . hunk session comment clear --repo . --yes [--file README.md] ``` -- `comment add` requires `--file`, `--summary`, and exactly one of `--old-line` or `--new-line` -- `comment add` reveals the note by default; pass `--no-reveal` to keep the current focus +- `comment add` is best for one note; `comment apply` is best when an agent already has several notes ready +- `comment add` requires `--file`, `--summary`, and exactly one of `--hunk`, `--old-line`, or `--new-line` +- `comment apply` payload items require `filePath`, `summary`, and one target such as `hunk`, `oldLine`, or `newLine` +- Prefer `--hunk ` when you want to annotate the whole diff hunk instead of picking a single line manually +- `comment add` and `comment apply` both keep the current focus by default; pass `--focus` when you want to jump to the new note or the first note in a batch +- `comment apply` reads a JSON batch from stdin and validates the full batch before mutating the live session - `comment list` and `comment clear` accept optional `--file` - Quote `--summary` and `--rationale` defensively in the shell @@ -119,13 +126,14 @@ Typical flow: 1. Load the right content (`reload` if needed) 2. Navigate to the first interesting file / hunk 3. Add a comment explaining what's happening and why -4. Move to the next point of interest -- repeat +4. If you already have several notes ready, prefer one `comment apply` batch over many separate shell invocations 5. Summarize when done Guidelines: - Work in the order that tells the clearest story, not necessarily file order - Navigate before commenting so the user sees the code you're discussing +- Use `comment apply` for agent-generated batches and `comment add` for one-off notes - Keep comments focused: intent, structure, risks, or follow-ups - Don't comment on every hunk -- highlight what the user wouldn't spot themselves @@ -138,3 +146,4 @@ Guidelines: - **"Pass the replacement Hunk command after `--`"** -- include `--` before the nested `diff` / `show` command. - **"Specify exactly one navigation target"** -- pick one of `--hunk`, `--old-line`, or `--new-line`. - **"Specify either --next-comment or --prev-comment, not both."** -- choose one comment-navigation direction. +- **"Pass --stdin to read batch comments from stdin JSON."** -- `comment apply` only reads its batch payload from stdin. diff --git a/src/core/cli.ts b/src/core/cli.ts index 5282ebb..9986e62 100644 --- a/src/core/cli.ts +++ b/src/core/cli.ts @@ -8,6 +8,7 @@ import type { LayoutMode, PagerCommandInput, ParsedCliInput, + SessionCommentApplyItemInput, } from "./types"; import { resolveCliVersion } from "./version"; @@ -192,6 +193,94 @@ function resolveJsonOutput(options: { json?: boolean }) { return options.json ? "json" : "text"; } +function parsePositiveJsonInt( + value: unknown, + { field, itemNumber }: { field: string; itemNumber: number }, +) { + if (value === undefined) { + return undefined; + } + + if (typeof value !== "number" || !Number.isInteger(value) || value <= 0) { + throw new Error(`Comment ${itemNumber} field \`${field}\` must be a positive integer.`); + } + + return value; +} + +function parseSessionCommentApplyPayload(raw: string): SessionCommentApplyItemInput[] { + if (raw.trim().length === 0) { + throw new Error("Session comment apply expected one JSON object on stdin."); + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + throw new Error("Session comment apply expected valid JSON on stdin."); + } + + if (!parsed || typeof parsed !== "object") { + throw new Error("Session comment apply expected one JSON object with a comments array."); + } + + const value = parsed as Record; + if (!Array.isArray(value.comments)) { + throw new Error("Session comment apply expected a top-level `comments` array."); + } + + return value.comments.map((comment, index) => { + const itemNumber = index + 1; + if (!comment || typeof comment !== "object") { + throw new Error(`Comment ${itemNumber} must be a JSON object.`); + } + + const item = comment as Record; + const filePath = item.filePath; + if (typeof filePath !== "string" || filePath.length === 0) { + throw new Error(`Comment ${itemNumber} requires a non-empty \`filePath\`.`); + } + + const summary = item.summary; + if (typeof summary !== "string" || summary.length === 0) { + throw new Error(`Comment ${itemNumber} requires a non-empty \`summary\`.`); + } + + const hunk = parsePositiveJsonInt(item.hunk, { field: "hunk", itemNumber }); + const hunkNumber = parsePositiveJsonInt(item.hunkNumber, { field: "hunkNumber", itemNumber }); + if (hunk !== undefined && hunkNumber !== undefined && hunk !== hunkNumber) { + throw new Error( + `Comment ${itemNumber} must not disagree between \`hunk\` and \`hunkNumber\`.`, + ); + } + + const oldLine = parsePositiveJsonInt(item.oldLine, { field: "oldLine", itemNumber }); + const newLine = parsePositiveJsonInt(item.newLine, { field: "newLine", itemNumber }); + const resolvedHunkNumber = hunk ?? hunkNumber; + + const selectors = [ + resolvedHunkNumber !== undefined, + oldLine !== undefined, + newLine !== undefined, + ].filter(Boolean); + if (selectors.length !== 1) { + throw new Error( + `Comment ${itemNumber} must specify exactly one of \`hunk\`, \`hunkNumber\`, \`oldLine\`, or \`newLine\`.`, + ); + } + + return { + filePath, + hunkNumber: resolvedHunkNumber, + side: oldLine !== undefined ? "old" : newLine !== undefined ? "new" : undefined, + line: oldLine ?? newLine, + summary, + rationale: typeof item.rationale === "string" ? item.rationale : undefined, + author: typeof item.author === "string" ? item.author : undefined, + }; + }); +} + /** Normalize one explicit session selector from either session id or repo root. */ function resolveExplicitSessionSelector( sessionId: string | undefined, @@ -484,7 +573,8 @@ async function parseSessionCommand(tokens: string[]): Promise { " hunk session navigate ( | --repo ) (--next-comment | --prev-comment)", " hunk session reload ( | --repo | --session-path ) [--source ] -- diff [ref] [-- ]", " hunk session reload ( | --repo | --session-path ) [--source ] -- show [ref] [-- ]", - " hunk session comment add ( | --repo ) --file (--old-line | --new-line ) --summary ", + " hunk session comment add ( | --repo ) --file (--hunk | --old-line | --new-line ) --summary [--focus]", + " hunk session comment apply ( | --repo ) --stdin [--focus]", " hunk session comment list ( | --repo )", " hunk session comment rm ( | --repo ) ", " hunk session comment clear ( | --repo ) --yes", @@ -728,7 +818,8 @@ async function parseSessionCommand(tokens: string[]): Promise { text: [ "Usage:", - " hunk session comment add ( | --repo ) --file (--old-line | --new-line ) --summary ", + " hunk session comment add ( | --repo ) --file (--hunk | --old-line | --new-line ) --summary [--focus]", + " hunk session comment apply ( | --repo ) --stdin [--focus]", " hunk session comment list ( | --repo ) [--file ]", " hunk session comment rm ( | --repo ) ", " hunk session comment clear ( | --repo ) [--file ] --yes", @@ -743,12 +834,12 @@ async function parseSessionCommand(tokens: string[]): Promise { .requiredOption("--file ", "diff file path as shown by Hunk") .requiredOption("--summary ", "short review note") .option("--repo ", "target the live session whose repo root matches this path") + .option("--hunk ", "1-based hunk number within the file", parsePositiveInt) .option("--old-line ", "1-based line number on the old side", parsePositiveInt) .option("--new-line ", "1-based line number on the new side", parsePositiveInt) .option("--rationale ", "optional longer explanation") .option("--author ", "optional author label") - .option("--reveal", "jump to and reveal the note") - .option("--no-reveal", "add the note without moving focus") + .option("--focus", "add the note and focus the viewport on it") .option("--json", "emit structured JSON"); let parsedSessionId: string | undefined; @@ -756,11 +847,12 @@ async function parseSessionCommand(tokens: string[]): Promise { repo?: string; file: string; summary: string; + hunk?: number; oldLine?: number; newLine?: number; rationale?: string; author?: string; - reveal?: boolean; + focus?: boolean; json?: boolean; } = { file: "", @@ -774,11 +866,12 @@ async function parseSessionCommand(tokens: string[]): Promise { repo?: string; file: string; summary: string; + hunk?: number; oldLine?: number; newLine?: number; rationale?: string; author?: string; - reveal?: boolean; + focus?: boolean; json?: boolean; }, ) => { @@ -794,11 +887,14 @@ async function parseSessionCommand(tokens: string[]): Promise { await parseStandaloneCommand(command, commentRest); const selectors = [ + parsedOptions.hunk !== undefined, parsedOptions.oldLine !== undefined, parsedOptions.newLine !== undefined, ].filter(Boolean); if (selectors.length !== 1) { - throw new Error("Specify exactly one comment target: --old-line or --new-line ."); + throw new Error( + "Specify exactly one comment target: --hunk , --old-line , or --new-line .", + ); } return { @@ -807,12 +903,92 @@ async function parseSessionCommand(tokens: string[]): Promise { output: resolveJsonOutput(parsedOptions), selector: resolveExplicitSessionSelector(parsedSessionId, parsedOptions.repo), filePath: parsedOptions.file, - side: parsedOptions.oldLine !== undefined ? "old" : "new", - line: parsedOptions.oldLine ?? parsedOptions.newLine ?? 0, + hunkNumber: parsedOptions.hunk, + side: + parsedOptions.oldLine !== undefined + ? "old" + : parsedOptions.newLine !== undefined + ? "new" + : undefined, + line: parsedOptions.oldLine ?? parsedOptions.newLine, summary: parsedOptions.summary, rationale: parsedOptions.rationale, author: parsedOptions.author, - reveal: parsedOptions.reveal ?? true, + reveal: parsedOptions.focus ?? false, + }; + } + + if (commentSubcommand === "apply") { + const command = new Command("session comment apply") + .description("apply many live inline review notes from stdin JSON") + .argument("[sessionId]") + .option("--repo ", "target the live session whose repo root matches this path") + .option("--stdin", "read the comment batch from stdin as JSON") + .option("--focus", "apply the batch and focus the first note") + .option("--json", "emit structured JSON"); + + let parsedSessionId: string | undefined; + let parsedOptions: { + repo?: string; + stdin?: boolean; + focus?: boolean; + json?: boolean; + } = {}; + + command.action( + ( + sessionId: string | undefined, + options: { + repo?: string; + stdin?: boolean; + focus?: boolean; + json?: boolean; + }, + ) => { + parsedSessionId = sessionId; + parsedOptions = options; + }, + ); + + if (commentRest.includes("--help") || commentRest.includes("-h")) { + return { + kind: "help", + text: + `${command.helpInformation().trimEnd()}\n\n` + + [ + "Stdin JSON shape:", + " {", + ' "comments": [', + " {", + ' "filePath": "README.md",', + ' "hunk": 2,', + ' "summary": "Explain this hunk",', + ' "rationale": "Optional detail",', + ' "author": "Pi"', + " }", + " ]", + " }", + ].join("\n") + + "\n", + }; + } + + await parseStandaloneCommand(command, commentRest); + if (!parsedOptions.stdin) { + throw new Error("Pass --stdin to read batch comments from stdin JSON."); + } + + const comments = parseSessionCommentApplyPayload( + await new Response(Bun.stdin.stream()).text(), + ); + + return { + kind: "session", + action: "comment-apply", + output: resolveJsonOutput(parsedOptions), + selector: resolveExplicitSessionSelector(parsedSessionId, parsedOptions.repo), + comments, + revealMode: parsedOptions.focus ? "first" : "none", }; } @@ -942,7 +1118,7 @@ async function parseSessionCommand(tokens: string[]): Promise { }; } - throw new Error("Supported comment subcommands are add, list, rm, and clear."); + throw new Error("Supported comment subcommands are add, apply, list, rm, and clear."); } throw new Error(`Unknown session command: ${subcommand}`); diff --git a/src/core/liveComments.ts b/src/core/liveComments.ts index f1712d2..6d86ee3 100644 --- a/src/core/liveComments.ts +++ b/src/core/liveComments.ts @@ -1,6 +1,12 @@ import type { Hunk } from "@pierre/diffs"; +import type { CommentTargetInput, DiffSide, LiveComment } from "../mcp/types"; import type { DiffFile } from "./types"; -import type { CommentToolInput, DiffSide, LiveComment } from "../mcp/types"; + +export interface ResolvedCommentTarget { + hunkIndex: number; + side: DiffSide; + line: number; +} /** Compute the inclusive old/new line spans touched by one hunk. */ export function hunkLineRange(hunk: Hunk) { @@ -33,9 +39,83 @@ export function findHunkIndexForLine(file: DiffFile, side: DiffSide, line: numbe }); } +/** Pick one stable anchor line inside a hunk, preferring new-side changes when present. */ +export function firstCommentTargetForHunk(hunk: Hunk): Omit { + let deletionLineNumber = hunk.deletionStart; + let additionLineNumber = hunk.additionStart; + let firstDeletionLine: number | undefined; + + for (const content of hunk.hunkContent) { + if (content.type === "context") { + deletionLineNumber += content.lines; + additionLineNumber += content.lines; + continue; + } + + if (content.additions > 0) { + return { + side: "new", + line: additionLineNumber, + }; + } + + if (content.deletions > 0 && firstDeletionLine === undefined) { + firstDeletionLine = deletionLineNumber; + } + + deletionLineNumber += content.deletions; + additionLineNumber += content.additions; + } + + if (firstDeletionLine !== undefined) { + return { + side: "old", + line: firstDeletionLine, + }; + } + + const fallbackRange = hunkLineRange(hunk); + return hunk.additionLines > 0 + ? { side: "new", line: fallbackRange.newRange[0] } + : { side: "old", line: fallbackRange.oldRange[0] }; +} + +/** Resolve a line-based or hunk-based live-comment target against one visible diff file. */ +export function resolveCommentTarget( + file: DiffFile, + input: CommentTargetInput, +): ResolvedCommentTarget { + if (input.hunkIndex !== undefined) { + const hunk = file.metadata.hunks[input.hunkIndex]; + if (!hunk) { + throw new Error(`No diff hunk ${input.hunkIndex + 1} exists in ${input.filePath}.`); + } + + return { + hunkIndex: input.hunkIndex, + ...firstCommentTargetForHunk(hunk), + }; + } + + if (!input.side || input.line === undefined) { + throw new Error("comment requires either hunkIndex or both side and line."); + } + + const hunkIndex = findHunkIndexForLine(file, input.side, input.line); + if (hunkIndex < 0) { + throw new Error(`No ${input.side} diff hunk in ${input.filePath} covers line ${input.line}.`); + } + + return { + hunkIndex, + side: input.side, + line: input.line, + }; +} + /** Convert one incoming MCP comment command into a live annotation. */ export function buildLiveComment( - input: CommentToolInput, + input: CommentTargetInput & { side: DiffSide; line: number }, commentId: string, createdAt: string, hunkIndex: number, diff --git a/src/core/types.ts b/src/core/types.ts index 13d4b28..43dac33 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -135,14 +135,34 @@ export interface SessionCommentAddCommandInput { output: SessionCommandOutput; selector: SessionSelectorInput; filePath: string; - side: "old" | "new"; - line: number; + hunkNumber?: number; + side?: "old" | "new"; + line?: number; summary: string; rationale?: string; author?: string; reveal: boolean; } +export interface SessionCommentApplyItemInput { + filePath: string; + hunkNumber?: number; + side?: "old" | "new"; + line?: number; + summary: string; + rationale?: string; + author?: string; +} + +export interface SessionCommentApplyCommandInput { + kind: "session"; + action: "comment-apply"; + output: SessionCommandOutput; + selector: SessionSelectorInput; + comments: SessionCommentApplyItemInput[]; + revealMode: "none" | "first"; +} + export interface SessionCommentListCommandInput { kind: "session"; action: "comment-list"; @@ -174,6 +194,7 @@ export type SessionCommandInput = | SessionNavigateCommandInput | SessionReloadCommandInput | SessionCommentAddCommandInput + | SessionCommentApplyCommandInput | SessionCommentListCommandInput | SessionCommentRemoveCommandInput | SessionCommentClearCommandInput; diff --git a/src/mcp/client.ts b/src/mcp/client.ts index 86fe88f..90c5a8d 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -1,4 +1,5 @@ import type { + AppliedCommentBatchResult, AppliedCommentResult, ClearedCommentsResult, HunkSessionRegistration, @@ -25,6 +26,9 @@ interface HunkAppBridge { applyComment: ( message: Extract, ) => Promise; + applyCommentBatch: ( + message: Extract, + ) => Promise; navigateToHunk: ( message: Extract, ) => Promise; @@ -261,6 +265,8 @@ export class HunkHostClient { switch (message.command) { case "comment": return this.bridge.applyComment(message); + case "comment_batch": + return this.bridge.applyCommentBatch(message); case "navigate_to_hunk": return this.bridge.navigateToHunk(message); case "reload_session": diff --git a/src/mcp/daemonState.ts b/src/mcp/daemonState.ts index 67d9bd1..aa6a694 100644 --- a/src/mcp/daemonState.ts +++ b/src/mcp/daemonState.ts @@ -1,8 +1,10 @@ import { randomUUID } from "node:crypto"; import type { + AppliedCommentBatchResult, AppliedCommentResult, ClearedCommentsResult, ClearCommentsToolInput, + CommentBatchToolInput, CommentToolInput, HunkSessionRegistration, HunkSessionSnapshot, @@ -284,6 +286,16 @@ export class HunkDaemonState { ); } + sendCommentBatch(input: CommentBatchToolInput) { + return this.sendCommand( + { sessionId: input.sessionId, sessionPath: input.sessionPath, repoRoot: input.repoRoot }, + "comment_batch", + input, + "Timed out waiting for the Hunk session to apply the comment batch.", + 30_000, + ); + } + sendNavigateToHunk(input: NavigateToHunkToolInput) { return this.sendCommand( { sessionId: input.sessionId, sessionPath: input.sessionPath, repoRoot: input.repoRoot }, diff --git a/src/mcp/server.ts b/src/mcp/server.ts index b8b02e6..fc3ccfc 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -22,6 +22,7 @@ const SUPPORTED_SESSION_ACTIONS: SessionDaemonAction[] = [ "navigate", "reload", "comment-add", + "comment-apply", "comment-list", "comment-rm", "comment-clear", @@ -122,11 +123,19 @@ async function handleSessionApiRequest(state: HunkDaemonState, request: Request) }), }; break; - case "comment-add": + case "comment-add": { + if ( + input.hunkNumber === undefined && + (input.side === undefined || input.line === undefined) + ) { + throw new Error("comment-add requires either hunkNumber or both side and line."); + } + response = { result: await state.sendComment({ ...input.selector, filePath: input.filePath, + hunkIndex: input.hunkNumber !== undefined ? input.hunkNumber - 1 : undefined, side: input.side, line: input.line, summary: input.summary, @@ -136,6 +145,24 @@ async function handleSessionApiRequest(state: HunkDaemonState, request: Request) }), }; break; + } + case "comment-apply": + response = { + result: await state.sendCommentBatch({ + ...input.selector, + comments: input.comments.map((comment) => ({ + filePath: comment.filePath, + hunkIndex: comment.hunkNumber !== undefined ? comment.hunkNumber - 1 : undefined, + side: comment.side, + line: comment.line, + summary: comment.summary, + rationale: comment.rationale, + author: comment.author, + })), + revealMode: input.revealMode, + }), + }; + break; case "comment-list": response = { comments: state.listComments(input.selector, { filePath: input.filePath }), diff --git a/src/mcp/types.ts b/src/mcp/types.ts index 59afd2a..43c90ba 100644 --- a/src/mcp/types.ts +++ b/src/mcp/types.ts @@ -63,16 +63,27 @@ export interface HunkSessionSnapshot { updatedAt: string; } -export interface CommentToolInput extends SessionTargetInput { +export interface CommentTargetInput { filePath: string; - side: DiffSide; - line: number; + hunkIndex?: number; + side?: DiffSide; + line?: number; summary: string; rationale?: string; - reveal?: boolean; author?: string; } +export interface CommentToolInput extends SessionTargetInput, CommentTargetInput { + reveal?: boolean; +} + +export interface CommentBatchItemInput extends CommentTargetInput {} + +export interface CommentBatchToolInput extends SessionTargetInput { + comments: CommentBatchItemInput[]; + revealMode?: "none" | "first"; +} + export interface NavigateToFileToolInput extends SessionTargetInput { filePath: string; hunkIndex?: number; @@ -123,6 +134,10 @@ export interface AppliedCommentResult { line: number; } +export interface AppliedCommentBatchResult { + applied: AppliedCommentResult[]; +} + export interface NavigatedSelectionResult { fileId: string; filePath: string; @@ -171,6 +186,7 @@ export interface SelectedSessionContext { export type SessionCommandResult = | AppliedCommentResult + | AppliedCommentBatchResult | NavigatedSelectionResult | RemovedCommentResult | ClearedCommentsResult @@ -223,6 +239,12 @@ export type SessionServerMessage = command: "comment"; input: CommentToolInput; } + | { + type: "command"; + requestId: string; + command: "comment_batch"; + input: CommentBatchToolInput; + } | { type: "command"; requestId: string; diff --git a/src/session/commands.ts b/src/session/commands.ts index 2ea14b5..2144c6b 100644 --- a/src/session/commands.ts +++ b/src/session/commands.ts @@ -3,6 +3,7 @@ import type { SessionCommandInput, SessionCommandOutput, SessionCommentAddCommandInput, + SessionCommentApplyCommandInput, SessionCommentClearCommandInput, SessionCommentListCommandInput, SessionCommentRemoveCommandInput, @@ -17,6 +18,7 @@ import { } from "../mcp/daemonLauncher"; import { resolveHunkMcpConfig } from "../mcp/config"; import type { + AppliedCommentBatchResult, AppliedCommentResult, ClearedCommentsResult, ListedSession, @@ -45,6 +47,7 @@ export interface HunkDaemonCliClient { navigateToHunk(input: SessionNavigateCommandInput): Promise; reloadSession(input: SessionReloadCommandInput): Promise; addComment(input: SessionCommentAddCommandInput): Promise; + applyComments(input: SessionCommentApplyCommandInput): Promise; listComments(input: SessionCommentListCommandInput): Promise; removeComment(input: SessionCommentRemoveCommandInput): Promise; clearComments(input: SessionCommentClearCommandInput): Promise; @@ -57,6 +60,7 @@ const REQUIRED_ACTION_BY_COMMAND: Record({ + action: "comment-apply", + selector: input.selector, + comments: input.comments, + revealMode: input.revealMode, + }) + ).result; + } + async listComments(input: SessionCommentListCommandInput) { return ( await this.request<{ comments: SessionLiveCommentSummary[] }>({ @@ -517,6 +533,24 @@ function formatCommentOutput(selector: SessionSelectorInput, result: AppliedComm return `Added live comment ${result.commentId} on ${result.filePath}:${result.line} (${result.side}) in hunk ${result.hunkIndex + 1} for ${formatSelector(selector)}.\n`; } +function formatCommentApplyOutput( + selector: SessionSelectorInput, + result: AppliedCommentBatchResult, +) { + if (result.applied.length === 0) { + return `Applied 0 live comments to ${formatSelector(selector)}.\n`; + } + + return `${[ + `Applied ${result.applied.length} live comments to ${formatSelector(selector)}:`, + ...result.applied.map( + (comment) => + ` - ${comment.commentId} on ${comment.filePath}:${comment.line} (${comment.side}) hunk ${comment.hunkIndex + 1}`, + ), + "", + ].join("\n")}`; +} + function formatCommentListOutput( selector: SessionSelectorInput, comments: SessionLiveCommentSummary[], @@ -640,6 +674,15 @@ export async function runSessionCommand(input: SessionCommandInput) { formatCommentOutput(input.selector, result), ); } + case "comment-apply": { + const result = await client.applyComments({ + ...input, + selector: normalizedSelector!, + }); + return renderOutput(input.output, { result }, () => + formatCommentApplyOutput(input.selector, result), + ); + } case "comment-list": { const comments = await client.listComments({ ...input, diff --git a/src/session/protocol.ts b/src/session/protocol.ts index 0d77dc8..f662193 100644 --- a/src/session/protocol.ts +++ b/src/session/protocol.ts @@ -1,5 +1,6 @@ import type { SessionCommentAddCommandInput, + SessionCommentApplyCommandInput, SessionCommentClearCommandInput, SessionCommentListCommandInput, SessionCommentRemoveCommandInput, @@ -8,6 +9,7 @@ import type { SessionSelectorInput, } from "../core/types"; import type { + AppliedCommentBatchResult, AppliedCommentResult, ClearedCommentsResult, ListedSession, @@ -29,6 +31,7 @@ export type SessionDaemonAction = | "navigate" | "reload" | "comment-add" + | "comment-apply" | "comment-list" | "comment-rm" | "comment-clear"; @@ -69,13 +72,20 @@ export type SessionDaemonRequest = action: "comment-add"; selector: SessionCommentAddCommandInput["selector"]; filePath: string; - side: "old" | "new"; - line: number; + hunkNumber?: number; + side?: "old" | "new"; + line?: number; summary: string; rationale?: string; author?: string; reveal: boolean; } + | { + action: "comment-apply"; + selector: SessionCommentApplyCommandInput["selector"]; + comments: SessionCommentApplyCommandInput["comments"]; + revealMode: SessionCommentApplyCommandInput["revealMode"]; + } | { action: "comment-list"; selector: SessionCommentListCommandInput["selector"]; @@ -99,6 +109,7 @@ export type SessionDaemonResponse = | { result: NavigatedSelectionResult } | { result: ReloadedSessionResult } | { result: AppliedCommentResult } + | { result: AppliedCommentBatchResult } | { comments: SessionLiveCommentSummary[] } | { result: RemovedCommentResult } | { result: ClearedCommentsResult }; diff --git a/src/ui/hooks/useHunkSessionBridge.ts b/src/ui/hooks/useHunkSessionBridge.ts index f6986e9..233288e 100644 --- a/src/ui/hooks/useHunkSessionBridge.ts +++ b/src/ui/hooks/useHunkSessionBridge.ts @@ -5,11 +5,13 @@ import { findDiffFileByPath, findHunkIndexForLine, hunkLineRange, + resolveCommentTarget, } from "../../core/liveComments"; import { HunkHostClient } from "../../mcp/client"; import { filterReviewFiles, mergeFileAnnotationsByFileId } from "../lib/files"; import { buildAnnotatedHunkCursors, findNextHunkCursor } from "../lib/hunks"; import type { + AppliedCommentBatchResult, LiveComment, ReloadedSessionResult, SessionLiveCommentSummary, @@ -157,19 +159,18 @@ export function useHunkSessionBridge({ throw new Error(`No visible diff file matches ${message.input.filePath}.`); } - const hunkIndex = findHunkIndexForLine(file, message.input.side, message.input.line); - if (hunkIndex < 0) { - throw new Error( - `No ${message.input.side} diff hunk in ${message.input.filePath} covers line ${message.input.line}.`, - ); - } + const target = resolveCommentTarget(file, message.input); const commentId = `mcp:${message.requestId}`; const liveComment = buildLiveComment( - message.input, + { + ...message.input, + side: target.side, + line: target.line, + }, commentId, new Date().toISOString(), - hunkIndex, + target.hunkIndex, ); setLiveCommentsByFileId((current) => ({ @@ -178,7 +179,7 @@ export function useHunkSessionBridge({ })); if (message.input.reveal ?? true) { - jumpToFile(file.id, hunkIndex); + jumpToFile(file.id, target.hunkIndex); openAgentNotes(); } @@ -186,9 +187,65 @@ export function useHunkSessionBridge({ commentId, fileId: file.id, filePath: file.path, - hunkIndex, - side: message.input.side, - line: message.input.line, + hunkIndex: target.hunkIndex, + side: target.side, + line: target.line, + }; + }, + [files, jumpToFile, openAgentNotes], + ); + + const applyIncomingCommentBatch = useCallback( + async ( + message: Extract, + ): Promise => { + const createdAt = new Date().toISOString(); + const prepared = message.input.comments.map((input, index) => { + const file = findDiffFileByPath(files, input.filePath); + if (!file) { + throw new Error(`No visible diff file matches ${input.filePath}.`); + } + + const target = resolveCommentTarget(file, input); + return { + file, + target, + liveComment: buildLiveComment( + { + ...input, + side: target.side, + line: target.line, + }, + `mcp:${message.requestId}:${index}`, + createdAt, + target.hunkIndex, + ), + }; + }); + + setLiveCommentsByFileId((current) => { + const next = { ...current }; + for (const entry of prepared) { + next[entry.file.id] = [...(next[entry.file.id] ?? []), entry.liveComment]; + } + return next; + }); + + if (message.input.revealMode === "first" && prepared.length > 0) { + const first = prepared[0]!; + jumpToFile(first.file.id, first.target.hunkIndex); + openAgentNotes(); + } + + return { + applied: prepared.map(({ file, target, liveComment }) => ({ + commentId: liveComment.id, + fileId: file.id, + filePath: file.path, + hunkIndex: target.hunkIndex, + side: target.side, + line: target.line, + })), }; }, [files, jumpToFile, openAgentNotes], @@ -286,6 +343,7 @@ export function useHunkSessionBridge({ hostClient.setBridge({ applyComment: applyIncomingComment, + applyCommentBatch: applyIncomingCommentBatch, navigateToHunk: navigateToHunkSelection, reloadSession: reloadIncomingSession, removeComment: removeIncomingComment, @@ -297,6 +355,7 @@ export function useHunkSessionBridge({ }; }, [ applyIncomingComment, + applyIncomingCommentBatch, clearIncomingComments, hostClient, navigateToHunkSelection, diff --git a/test/cli.test.ts b/test/cli.test.ts index 71bced1..cab4bfd 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -306,7 +306,7 @@ describe("parseCli", () => { ).rejects.toThrow("Pass the replacement Hunk command after `--`"); }); - test("parses session comment add", async () => { + test("parses session comment add without focusing by default", async () => { const parsed = await parseCli([ "bun", "hunk", @@ -324,7 +324,6 @@ describe("parseCli", () => { "Live review is the main value.", "--author", "Pi", - "--no-reveal", ]); expect(parsed).toEqual({ @@ -342,6 +341,115 @@ describe("parseCli", () => { }); }); + test("parses session comment add by hunk number", async () => { + const parsed = await parseCli([ + "bun", + "hunk", + "session", + "comment", + "add", + "session-1", + "--file", + "README.md", + "--hunk", + "2", + "--summary", + "Anchor this note to the whole hunk", + "--json", + ]); + + expect(parsed).toEqual({ + kind: "session", + action: "comment-add", + selector: { sessionId: "session-1" }, + filePath: "README.md", + hunkNumber: 2, + side: undefined, + line: undefined, + summary: "Anchor this note to the whole hunk", + rationale: undefined, + author: undefined, + reveal: false, + output: "json", + }); + }); + + test("parses session comment add with --focus", async () => { + const parsed = await parseCli([ + "bun", + "hunk", + "session", + "comment", + "add", + "session-1", + "--file", + "README.md", + "--new-line", + "103", + "--summary", + "Frame this as MCP-first", + "--focus", + ]); + + expect(parsed).toEqual({ + kind: "session", + action: "comment-add", + selector: { sessionId: "session-1" }, + filePath: "README.md", + side: "new", + line: 103, + summary: "Frame this as MCP-first", + reveal: true, + output: "text", + }); + }); + + test("parses session comment apply with --focus", async () => { + const originalStdin = Bun.stdin.stream; + Bun.stdin.stream = () => + new ReadableStream({ + start(controller) { + controller.enqueue( + new TextEncoder().encode( + '{"comments":[{"filePath":"README.md","hunk":2,"summary":"Explain this hunk"}]}', + ), + ); + controller.close(); + }, + }); + + try { + const parsed = await parseCli([ + "bun", + "hunk", + "session", + "comment", + "apply", + "session-1", + "--stdin", + "--focus", + "--json", + ]); + + expect(parsed).toEqual({ + kind: "session", + action: "comment-apply", + selector: { sessionId: "session-1" }, + comments: [ + { + filePath: "README.md", + hunkNumber: 2, + summary: "Explain this hunk", + }, + ], + revealMode: "first", + output: "json", + }); + } finally { + Bun.stdin.stream = originalStdin; + } + }); + test("parses session comment list with file filter", async () => { const parsed = await parseCli([ "bun", @@ -491,6 +599,27 @@ describe("parseCli", () => { ).rejects.toThrow("Specify exactly one navigation target"); }); + test("rejects session comment add with multiple target selectors", async () => { + await expect( + parseCli([ + "bun", + "hunk", + "session", + "comment", + "add", + "session-1", + "--file", + "README.md", + "--hunk", + "2", + "--new-line", + "103", + "--summary", + "Too many targets", + ]), + ).rejects.toThrow("Specify exactly one comment target"); + }); + test("rejects session comment clear without confirmation", async () => { await expect( parseCli(["bun", "hunk", "session", "comment", "clear", "session-1"]), diff --git a/test/help-output.test.ts b/test/help-output.test.ts index 1a7d1cc..cd98178 100644 --- a/test/help-output.test.ts +++ b/test/help-output.test.ts @@ -70,6 +70,28 @@ describe("CLI help output", () => { expect(stdout).not.toContain("\u001b[?1049h"); }); + test("prints session comment apply help with --focus wording", () => { + const proc = Bun.spawnSync( + ["bun", "run", "src/main.tsx", "session", "comment", "apply", "--help"], + { + cwd: process.cwd(), + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }, + ); + + const stdout = Buffer.from(proc.stdout).toString("utf8"); + const stderr = Buffer.from(proc.stderr).toString("utf8"); + + expect(proc.exitCode).toBe(0); + expect(stderr).toBe(""); + expect(stdout).toContain("Usage: session comment apply"); + expect(stdout).toContain("--focus"); + expect(stdout).not.toContain("--reveal-last"); + expect(stdout).not.toContain("\u001b[?1049h"); + }); + test("prints the package version for --version without terminal takeover sequences", () => { const expectedVersion = require("../package.json").version; const proc = Bun.spawnSync(["bun", "run", "src/main.tsx", "--version"], { diff --git a/test/live-comments.test.ts b/test/live-comments.test.ts index 09530dc..9554ee7 100644 --- a/test/live-comments.test.ts +++ b/test/live-comments.test.ts @@ -4,7 +4,9 @@ import { buildLiveComment, findDiffFileByPath, findHunkIndexForLine, + firstCommentTargetForHunk, hunkLineRange, + resolveCommentTarget, } from "../src/core/liveComments"; import type { DiffFile } from "../src/core/types"; @@ -58,6 +60,40 @@ function createDiffFile(): DiffFile { }; } +function createLateChangeDiffFile(): DiffFile { + const beforeLines = Array.from( + { length: 10 }, + (_, index) => `export const line${index + 1} = ${index + 1};`, + ); + const afterLines = [...beforeLines]; + afterLines[9] = "export const line10 = 100;"; + + const metadata = parseDiffFromFile( + { + name: "src/late.ts", + contents: `${beforeLines.join("\n")}\n`, + cacheKey: "late-before", + }, + { + name: "src/late.ts", + contents: `${afterLines.join("\n")}\n`, + cacheKey: "late-after", + }, + { context: 3 }, + true, + ); + + return { + id: "file:late", + path: "src/late.ts", + patch: "", + language: "typescript", + stats: { additions: 1, deletions: 1 }, + metadata, + agent: null, + }; +} + describe("live comment helpers", () => { test("finds a diff file by current or previous path", () => { const file = createDiffFile(); @@ -75,6 +111,40 @@ describe("live comment helpers", () => { expect(findHunkIndexForLine(file, "new", 40)).toBe(-1); }); + test("resolves hunk-targeted comments to the first changed line on the preferred side", () => { + const file = createLateChangeDiffFile(); + const target = resolveCommentTarget(file, { + filePath: "src/late.ts", + hunkIndex: 0, + summary: "Late change", + }); + + expect(target).toEqual({ + hunkIndex: 0, + side: "new", + line: 10, + }); + }); + + test("prefers a later addition over an earlier deletion-only chunk", () => { + const target = firstCommentTargetForHunk({ + additionStart: 20, + additionLines: 1, + deletionStart: 20, + deletionLines: 1, + hunkContent: [ + { type: "change", deletions: 1, additions: 0 }, + { type: "context", lines: 2 }, + { type: "change", deletions: 0, additions: 1 }, + ], + } as Parameters[0]); + + expect(target).toEqual({ + side: "new", + line: 22, + }); + }); + test("builds a live MCP comment annotation", () => { const comment = buildLiveComment( { diff --git a/test/mcp-daemon.test.ts b/test/mcp-daemon.test.ts index 852ab73..3889d99 100644 --- a/test/mcp-daemon.test.ts +++ b/test/mcp-daemon.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { HunkDaemonState, resolveSessionTarget } from "../src/mcp/daemonState"; import type { + AppliedCommentBatchResult, AppliedCommentResult, ClearedCommentsResult, HunkSessionRegistration, @@ -244,6 +245,71 @@ describe("Hunk MCP daemon state", () => { await expect(pending).resolves.toEqual(result); }); + test("routes comment-batch commands to the live session and resolves the async result", async () => { + const state = new HunkDaemonState(); + const sent: string[] = []; + const socket = { + send(data: string) { + sent.push(data); + }, + }; + + state.registerSession(socket, createRegistration(), createSnapshot()); + + const pending = state.sendCommentBatch({ + sessionId: "session-1", + comments: [ + { + filePath: "src/example.ts", + hunkIndex: 0, + summary: "Review note 1", + }, + { + filePath: "src/example.ts", + hunkIndex: 0, + summary: "Review note 2", + }, + ], + revealMode: "none", + }); + + expect(sent).toHaveLength(1); + const outgoing = JSON.parse(sent[0]!) as { + requestId: string; + command: string; + }; + expect(outgoing.command).toBe("comment_batch"); + + const result: AppliedCommentBatchResult = { + applied: [ + { + commentId: "comment-1", + fileId: "file-1", + filePath: "src/example.ts", + hunkIndex: 0, + side: "new", + line: 4, + }, + { + commentId: "comment-2", + fileId: "file-1", + filePath: "src/example.ts", + hunkIndex: 0, + side: "new", + line: 9, + }, + ], + }; + + state.handleCommandResult({ + requestId: outgoing.requestId, + ok: true, + result, + }); + + await expect(pending).resolves.toEqual(result); + }); + test("routes navigation commands to the live session and resolves the async result", async () => { const state = new HunkDaemonState(); const sent: string[] = []; diff --git a/test/mcp-e2e.test.ts b/test/mcp-e2e.test.ts index a5ef01f..8457513 100644 --- a/test/mcp-e2e.test.ts +++ b/test/mcp-e2e.test.ts @@ -131,6 +131,19 @@ function runSessionCli(args: string[], port: number) { return { proc, stdout, stderr }; } +async function reserveLoopbackPort() { + const listener = createServer(() => undefined); + await new Promise((resolve, reject) => { + listener.once("error", reject); + listener.listen(0, "127.0.0.1", () => resolve()); + }); + + const address = listener.address(); + const port = typeof address === "object" && address ? address.port : 0; + await new Promise((resolve) => listener.close(() => resolve())); + return port; +} + async function waitUntil( label: string, fn: () => Promise | T | null, @@ -183,7 +196,7 @@ describe("live session end-to-end", () => { ["export const alpha = 1;", "export const keep = true;"], ["export const alpha = 2;", "export const keep = true;", "export const gamma = true;"], ); - const port = 48000 + Math.floor(Math.random() * 1000); + const port = await reserveLoopbackPort(); const hunkProc = spawnHunkSession(fixture, { port }); let daemonPid: number | null = null; @@ -221,6 +234,7 @@ describe("live session end-to-end", () => { "Injected after the Hunk session auto-started the local daemon.", "--author", "Pi", + "--focus", "--json", ], port, @@ -286,7 +300,7 @@ describe("live session end-to-end", () => { "export const line10 = 110;", ], ); - const port = 48500 + Math.floor(Math.random() * 1000); + const port = await reserveLoopbackPort(); const hunkProc = spawnHunkSession(fixture, { port, quitAfterSeconds: 14, timeoutSeconds: 16 }); let daemonPid: number | null = null; @@ -377,7 +391,7 @@ describe("live session end-to-end", () => { ["export const beta = 1;", "export const shared = true;"], ["export const beta = 2;", "export const shared = true;", "export const onlyBeta = true;"], ); - const port = 49000 + Math.floor(Math.random() * 1000); + const port = await reserveLoopbackPort(); const hunkProcA = spawnHunkSession(fixtureA, { port, quitAfterSeconds: 10, @@ -428,6 +442,7 @@ describe("live session end-to-end", () => { "Alpha note", "--rationale", "Delivered only to the alpha Hunk session.", + "--focus", ], port, ); @@ -446,6 +461,7 @@ describe("live session end-to-end", () => { "Beta note", "--rationale", "Delivered only to the beta Hunk session.", + "--focus", ], port, ); diff --git a/test/mcp-server.test.ts b/test/mcp-server.test.ts index 7c61c61..99272f4 100644 --- a/test/mcp-server.test.ts +++ b/test/mcp-server.test.ts @@ -214,6 +214,7 @@ describe("Hunk session daemon server", () => { "navigate", "reload", "comment-add", + "comment-apply", "comment-list", "comment-rm", "comment-clear", @@ -362,4 +363,160 @@ describe("Hunk session daemon server", () => { server.stop(true); } }); + + test("forwards hunk-targeted comment requests through the session API", async () => { + const port = await reserveLoopbackPort(); + process.env.HUNK_MCP_HOST = "127.0.0.1"; + process.env.HUNK_MCP_PORT = String(port); + + const original = HunkDaemonState.prototype.sendComment; + HunkDaemonState.prototype.sendComment = function (input) { + expect(input).toMatchObject({ + sessionId: "session-1", + filePath: "src/example.ts", + hunkIndex: 1, + summary: "Whole-hunk note", + author: "Pi", + reveal: false, + }); + expect(input.side).toBeUndefined(); + expect(input.line).toBeUndefined(); + + return Promise.resolve({ + commentId: "comment-1", + fileId: "file-1", + filePath: "src/example.ts", + hunkIndex: 1, + side: "new", + line: 13, + }); + }; + + const server = serveHunkMcpServer(); + + try { + const response = await fetch(`http://127.0.0.1:${port}/session-api`, { + method: "POST", + headers: { + "content-type": "application/json", + }, + body: JSON.stringify({ + action: "comment-add", + selector: { sessionId: "session-1" }, + filePath: "src/example.ts", + hunkNumber: 2, + summary: "Whole-hunk note", + author: "Pi", + reveal: false, + }), + }); + + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ + result: { + commentId: "comment-1", + hunkIndex: 1, + side: "new", + line: 13, + }, + }); + } finally { + HunkDaemonState.prototype.sendComment = original; + server.stop(true); + } + }); + + test("forwards comment batches through the session API", async () => { + const port = await reserveLoopbackPort(); + process.env.HUNK_MCP_HOST = "127.0.0.1"; + process.env.HUNK_MCP_PORT = String(port); + + const original = HunkDaemonState.prototype.sendCommentBatch; + HunkDaemonState.prototype.sendCommentBatch = function (input) { + expect(input).toMatchObject({ + sessionId: "session-1", + revealMode: "none", + comments: [ + { + filePath: "src/example.ts", + hunkIndex: 0, + summary: "First", + author: "Pi", + }, + { + filePath: "src/example.ts", + hunkIndex: 1, + summary: "Second", + rationale: "Applied together.", + author: "Pi", + }, + ], + }); + + return Promise.resolve({ + applied: [ + { + commentId: "comment-1", + fileId: "file-1", + filePath: "src/example.ts", + hunkIndex: 0, + side: "new", + line: 2, + }, + { + commentId: "comment-2", + fileId: "file-1", + filePath: "src/example.ts", + hunkIndex: 1, + side: "new", + line: 13, + }, + ], + }); + }; + + const server = serveHunkMcpServer(); + + try { + const response = await fetch(`http://127.0.0.1:${port}/session-api`, { + method: "POST", + headers: { + "content-type": "application/json", + }, + body: JSON.stringify({ + action: "comment-apply", + selector: { sessionId: "session-1" }, + revealMode: "none", + comments: [ + { + filePath: "src/example.ts", + hunkNumber: 1, + summary: "First", + author: "Pi", + }, + { + filePath: "src/example.ts", + hunkNumber: 2, + summary: "Second", + rationale: "Applied together.", + author: "Pi", + }, + ], + }), + }); + + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ + result: { + applied: [ + { commentId: "comment-1", hunkIndex: 0, side: "new", line: 2 }, + { commentId: "comment-2", hunkIndex: 1, side: "new", line: 13 }, + ], + }, + }); + } finally { + HunkDaemonState.prototype.sendCommentBatch = original; + server.stop(true); + } + }); }); diff --git a/test/session-cli.test.ts b/test/session-cli.test.ts index 1c1a6de..cb171a0 100644 --- a/test/session-cli.test.ts +++ b/test/session-cli.test.ts @@ -80,8 +80,8 @@ function spawnHunkSession( fixture: ReturnType, { port, - quitAfterSeconds = 8, - timeoutSeconds = 10, + quitAfterSeconds = 12, + timeoutSeconds = 15, }: { port: number; quitAfterSeconds?: number; @@ -107,10 +107,10 @@ function spawnHunkSession( }); } -function runSessionCli(args: string[], port: number) { +function runSessionCli(args: string[], port: number, stdinText?: string) { const proc = Bun.spawnSync(["bun", "run", "src/main.tsx", "session", ...args], { cwd: repoRoot, - stdin: "ignore", + stdin: stdinText === undefined ? "ignore" : Buffer.from(stdinText), stdout: "pipe", stderr: "pipe", env: { @@ -347,8 +347,8 @@ describe("session CLI", () => { sessionId, "--file", fixture.afterName, - "--new-line", - "10", + "--hunk", + "2", "--summary", "Second hunk note", "--rationale", @@ -375,7 +375,7 @@ describe("session CLI", () => { filePath: fixture.afterName, hunkIndex: 1, side: "new", - line: 10, + line: 13, }, }); @@ -385,4 +385,250 @@ describe("session CLI", () => { await session.exited; } }, 20_000); + + test("comment apply adds a batch from stdin without moving focus by default", async () => { + if (!ttyToolsAvailable) { + return; + } + + const port = 48964; + const fixture = createFixtureFiles( + "apply-batch", + [ + "export const one = 1;", + "export const two = 2;", + "export const three = 3;", + "export const four = 4;", + "export const five = 5;", + "export const six = 6;", + "export const seven = 7;", + "export const eight = 8;", + "export const nine = 9;", + "export const ten = 10;", + "export const eleven = 11;", + "export const twelve = 12;", + "export const thirteen = 13;", + ], + [ + "export const one = 1;", + "export const two = 20;", + "export const three = 3;", + "export const four = 4;", + "export const five = 5;", + "export const six = 6;", + "export const seven = 7;", + "export const eight = 8;", + "export const nine = 9;", + "export const ten = 10;", + "export const eleven = 11;", + "export const twelve = 12;", + "export const thirteen = 130;", + ], + ); + const session = spawnHunkSession(fixture, { port, quitAfterSeconds: 18, timeoutSeconds: 20 }); + + try { + const listed = await waitUntil("registered live session", () => { + const { proc, stdout } = runSessionCli(["list", "--json"], port); + if (proc.exitCode !== 0) { + return null; + } + + const parsed = JSON.parse(stdout) as SessionListJson; + return parsed.sessions.length > 0 ? parsed.sessions : null; + }); + + const sessionId = listed[0]!.sessionId; + const apply = runSessionCli( + ["comment", "apply", sessionId, "--stdin", "--json"], + port, + JSON.stringify({ + comments: [ + { + filePath: fixture.afterName, + hunk: 1, + summary: "First hunk note", + author: "Pi", + }, + { + filePath: fixture.afterName, + hunk: 2, + summary: "Second hunk note", + rationale: "Applied in one batch.", + author: "Pi", + }, + ], + }), + ); + + expect(apply.proc.exitCode).toBe(0); + expect(apply.stderr).toBe(""); + const applied = JSON.parse(apply.stdout) as { + result?: { + applied?: Array<{ + filePath?: string; + hunkIndex?: number; + side?: string; + line?: number; + }>; + }; + }; + expect(applied).toMatchObject({ + result: { + applied: [ + { + filePath: fixture.afterName, + hunkIndex: 0, + side: "new", + line: 2, + }, + { + filePath: fixture.afterName, + hunkIndex: 1, + side: "new", + line: 13, + }, + ], + }, + }); + + const context = runSessionCli(["context", sessionId, "--json"], port); + expect(context.proc.exitCode).toBe(0); + expect(JSON.parse(context.stdout)).toMatchObject({ + context: { + selectedHunk: { + index: 0, + }, + }, + }); + + const comments = runSessionCli(["comment", "list", sessionId, "--json"], port); + expect(comments.proc.exitCode).toBe(0); + expect(JSON.parse(comments.stdout)).toMatchObject({ + comments: [{ summary: "First hunk note" }, { summary: "Second hunk note" }], + }); + } finally { + session.kill(); + await session.exited; + } + }, 20_000); + + test("comment apply with --focus jumps to the first applied comment", async () => { + if (!ttyToolsAvailable) { + return; + } + + const port = 48965; + const fixture = createFixtureFiles( + "apply-batch-focus", + [ + "export const one = 1;", + "export const two = 2;", + "export const three = 3;", + "export const four = 4;", + "export const five = 5;", + "export const six = 6;", + "export const seven = 7;", + "export const eight = 8;", + "export const nine = 9;", + "export const ten = 10;", + "export const eleven = 11;", + "export const twelve = 12;", + "export const thirteen = 13;", + ], + [ + "export const one = 1;", + "export const two = 20;", + "export const three = 3;", + "export const four = 4;", + "export const five = 5;", + "export const six = 6;", + "export const seven = 7;", + "export const eight = 8;", + "export const nine = 9;", + "export const ten = 10;", + "export const eleven = 11;", + "export const twelve = 12;", + "export const thirteen = 130;", + ], + ); + const session = spawnHunkSession(fixture, { port, quitAfterSeconds: 18, timeoutSeconds: 20 }); + + try { + const listed = await waitUntil("registered live session", () => { + const { proc, stdout } = runSessionCli(["list", "--json"], port); + if (proc.exitCode !== 0) { + return null; + } + + const parsed = JSON.parse(stdout) as SessionListJson; + return parsed.sessions.length > 0 ? parsed.sessions : null; + }); + + const sessionId = listed[0]!.sessionId; + const navigate = runSessionCli( + ["navigate", sessionId, "--file", fixture.afterName, "--hunk", "2", "--json"], + port, + ); + expect(navigate.proc.exitCode).toBe(0); + + await waitUntil("second hunk selection", () => { + const context = runSessionCli(["context", sessionId, "--json"], port); + if (context.proc.exitCode !== 0) { + return null; + } + + const parsed = JSON.parse(context.stdout) as { + context?: { selectedHunk?: { index: number } }; + }; + return parsed.context?.selectedHunk?.index === 1 ? parsed : null; + }); + + const apply = runSessionCli( + ["comment", "apply", sessionId, "--stdin", "--focus", "--json"], + port, + JSON.stringify({ + comments: [ + { + filePath: fixture.afterName, + hunk: 1, + summary: "First hunk note", + author: "Pi", + }, + { + filePath: fixture.afterName, + hunk: 2, + summary: "Second hunk note", + author: "Pi", + }, + ], + }), + ); + expect(apply.proc.exitCode).toBe(0); + expect(apply.stderr).toBe(""); + + const focused = await waitUntil("first hunk selection after focused batch apply", () => { + const context = runSessionCli(["context", sessionId, "--json"], port); + if (context.proc.exitCode !== 0) { + return null; + } + + const parsed = JSON.parse(context.stdout) as { + context?: { selectedHunk?: { index: number } }; + }; + return parsed.context?.selectedHunk?.index === 0 ? parsed : null; + }); + + expect(focused).toMatchObject({ + context: { + selectedHunk: { + index: 0, + }, + }, + }); + } finally { + session.kill(); + await session.exited; + } + }, 20_000); }); diff --git a/test/session-commands.test.ts b/test/session-commands.test.ts index c27b103..57d9804 100644 --- a/test/session-commands.test.ts +++ b/test/session-commands.test.ts @@ -52,6 +52,7 @@ function createClient(overrides: Partial): HunkDaemonCliCli "navigate", "reload", "comment-add", + "comment-apply", "comment-list", "comment-rm", "comment-clear", @@ -102,6 +103,18 @@ function createClient(overrides: Partial): HunkDaemonCliCli side: "new", line: 1, }), + applyComments: async () => ({ + applied: [ + { + commentId: "comment-1", + fileId: "file-1", + filePath: "README.md", + hunkIndex: 0, + side: "new", + line: 1, + }, + ], + }), listComments: async () => [], removeComment: async () => ({ commentId: "comment-1", @@ -255,6 +268,99 @@ describe("session command compatibility checks", () => { }); }); + test("runs comment-apply through the daemon and returns the applied batch", async () => { + setSessionCommandTestHooks({ + createClient: () => + createClient({ + applyComments: async (input) => { + expect(input.selector).toEqual({ sessionId: "session-1" }); + expect(input.revealMode).toBe("none"); + expect(input.comments).toEqual([ + { + filePath: "README.md", + hunkNumber: 2, + summary: "Explain this hunk", + author: "Pi", + }, + { + filePath: "README.md", + side: "new", + line: 103, + summary: "Tighten this wording", + }, + ]); + + return { + applied: [ + { + commentId: "comment-1", + fileId: "file-1", + filePath: "README.md", + hunkIndex: 1, + side: "new", + line: 12, + }, + { + commentId: "comment-2", + fileId: "file-1", + filePath: "README.md", + hunkIndex: 1, + side: "new", + line: 103, + }, + ], + }; + }, + }), + resolveDaemonAvailability: async () => true, + }); + + const output = await runSessionCommand({ + kind: "session", + action: "comment-apply", + selector: { sessionId: "session-1" }, + comments: [ + { + filePath: "README.md", + hunkNumber: 2, + summary: "Explain this hunk", + author: "Pi", + }, + { + filePath: "README.md", + side: "new", + line: 103, + summary: "Tighten this wording", + }, + ], + revealMode: "none", + output: "json", + } satisfies SessionCommandInput); + + expect(JSON.parse(output)).toEqual({ + result: { + applied: [ + { + commentId: "comment-1", + fileId: "file-1", + filePath: "README.md", + hunkIndex: 1, + side: "new", + line: 12, + }, + { + commentId: "comment-2", + fileId: "file-1", + filePath: "README.md", + hunkIndex: 1, + side: "new", + line: 103, + }, + ], + }, + }); + }); + test("passes a separate source path through reload commands", async () => { setSessionCommandTestHooks({ createClient: () => @@ -323,6 +429,7 @@ describe("session command compatibility checks", () => { "navigate", "reload", "comment-add", + "comment-apply", "comment-list", "comment-rm", "comment-clear",