From c22dd0e5c05cbcf086f20f4f6d2befee77d025f7 Mon Sep 17 00:00:00 2001
From: Tisankan Jeyakumar <tisankanvc@gmail.com>
Date: Sat, 29 Aug 2026 20:51:07 +0530
Subject: [PATCH] fix(session): reject ambiguous navigate targets

hunk session navigate accepted two selectors at once. A direction such as
--next-comment combined with --file, --hunk, or a line silently won, and the
absolute target was dropped without a word. The daemon also accepted a hunk or
line target with no filePath, which the live terminal cannot resolve.

Reject both combinations in the CLI parser and at the broker boundary. Keep the
documented precedence where exact line coordinates win over a hunk number on a
raw API request, and cover it with a test so the precedence stays explicit.

Closes #897
---
 .changeset/tidy-moons-invite.md               |  5 ++
 src/app/cli.test.ts                           | 26 ++++++++
 src/app/cli.ts                                | 20 +++---
 .../broker/brokerServer.helpers.test.ts       | 61 +++++++++++++++++++
 src/session/broker/brokerServer.ts            | 17 ++++++
 5 files changed, 120 insertions(+), 9 deletions(-)
 create mode 100644 .changeset/tidy-moons-invite.md

diff --git a/.changeset/tidy-moons-invite.md b/.changeset/tidy-moons-invite.md
new file mode 100644
index 000000000..ea5dab854
--- /dev/null
+++ b/.changeset/tidy-moons-invite.md
@@ -0,0 +1,5 @@
+---
+"hunkdiff": patch
+---
+
+Reject ambiguous `hunk session navigate` requests instead of silently ignoring the extra target.
diff --git a/src/app/cli.test.ts b/src/app/cli.test.ts
index dd9333691..e2a84b457 100644
--- a/src/app/cli.test.ts
+++ b/src/app/cli.test.ts
@@ -1432,6 +1432,32 @@ describe("parseCli", () => {
     }
   });
 
+  test("rejects session navigate when a comment direction is combined with an absolute target", async () => {
+    const conflictingOptions = [
+      ["--file", "README.md"],
+      ["--hunk", "1"],
+      ["--old-line", "10"],
+      ["--new-line", "10"],
+      ["--file", "README.md", "--hunk", "2"],
+    ];
+
+    for (const direction of ["--next-comment", "--prev-comment"]) {
+      for (const conflictingOption of conflictingOptions) {
+        await expect(
+          parseCli([
+            "bun",
+            "hunk",
+            "session",
+            "navigate",
+            "session-1",
+            direction,
+            ...conflictingOption,
+          ]),
+        ).rejects.toThrow("Specify exactly one navigation selector");
+      }
+    }
+  });
+
   test("rejects session navigate with both --next-comment and --prev-comment", async () => {
     await expect(
       parseCli([
diff --git a/src/app/cli.ts b/src/app/cli.ts
index 4c6d4483b..e02890ea1 100644
--- a/src/app/cli.ts
+++ b/src/app/cli.ts
@@ -1159,16 +1159,18 @@ async function parseSessionNavigateCommand(tokens: string[]): Promise<ParsedCliI
 
   await parseStandaloneCommand(command, tokens);
 
-  // A comment id is resolved by the daemon and must not silently discard another selector.
+  // A comment id or a direction is resolved by the daemon and must not silently discard another
+  // selector, so every selector combination is rejected rather than ranked.
+  const hasAbsoluteTarget =
+    parsedOptions.file !== undefined ||
+    parsedOptions.hunk !== undefined ||
+    parsedOptions.oldLine !== undefined ||
+    parsedOptions.newLine !== undefined;
+  const hasCommentDirection =
+    parsedOptions.nextComment === true || parsedOptions.prevComment === true;
   const commentHasConflictingSelector =
-    parsedOptions.comment !== undefined &&
-    (parsedOptions.file !== undefined ||
-      parsedOptions.hunk !== undefined ||
-      parsedOptions.oldLine !== undefined ||
-      parsedOptions.newLine !== undefined ||
-      parsedOptions.nextComment === true ||
-      parsedOptions.prevComment === true);
-  if (commentHasConflictingSelector) {
+    parsedOptions.comment !== undefined && (hasAbsoluteTarget || hasCommentDirection);
+  if (commentHasConflictingSelector || (hasCommentDirection && hasAbsoluteTarget)) {
     throw new Error(
       "Specify exactly one navigation selector: --comment, --next-comment / --prev-comment, or --file with a navigation target.",
     );
diff --git a/src/session/broker/brokerServer.helpers.test.ts b/src/session/broker/brokerServer.helpers.test.ts
index 9a5d3f7f1..4e22fe17e 100644
--- a/src/session/broker/brokerServer.helpers.test.ts
+++ b/src/session/broker/brokerServer.helpers.test.ts
@@ -278,6 +278,7 @@ describe("handleSessionApiRequest", () => {
       apiRequest({
         action: "navigate",
         selector: { sessionId: "s-1" },
+        filePath: "a.ts",
         hunkNumber: 2,
       } as SessionDaemonRequest),
     );
@@ -385,6 +386,66 @@ describe("handleSessionApiRequest", () => {
     });
   });
 
+  test("rejects a comment direction combined with another navigation target", async () => {
+    const { state } = createFakeState();
+    const response = await handleSessionApiRequest(
+      state,
+      apiRequest({
+        action: "navigate",
+        selector: { sessionId: "s-1" },
+        commentDirection: "next",
+        filePath: "a.ts",
+        hunkNumber: 2,
+      } as SessionDaemonRequest),
+    );
+
+    expect(response.status).toBe(400);
+    expect(await response.json()).toMatchObject({
+      error: expect.stringContaining("commentDirection cannot be combined"),
+    });
+  });
+
+  test("rejects a hunk or line target without a file path", async () => {
+    const { state } = createFakeState();
+    for (const target of [{ hunkNumber: 2 }, { side: "new" as const, line: 12 }]) {
+      const response = await handleSessionApiRequest(
+        state,
+        apiRequest({
+          action: "navigate",
+          selector: { sessionId: "s-1" },
+          ...target,
+        } as SessionDaemonRequest),
+      );
+
+      expect(response.status).toBe(400);
+      expect(await response.json()).toMatchObject({
+        error: expect.stringContaining("requires filePath"),
+      });
+    }
+  });
+
+  test("prefers exact line coordinates when a hunk number is also supplied", async () => {
+    const { state, calls } = createFakeState();
+    const response = await handleSessionApiRequest(
+      state,
+      apiRequest({
+        action: "navigate",
+        selector: { sessionId: "s-1" },
+        filePath: "a.ts",
+        hunkNumber: 2,
+        side: "new",
+        line: 12,
+      } as SessionDaemonRequest),
+    );
+
+    expect(response.status).toBe(200);
+    const dispatch = calls.find((c) => c.method === "dispatchCommand");
+    expect(dispatch).toBeDefined();
+    expect((dispatch!.args[0] as { input: Record<string, unknown> }).input).toMatchObject({
+      line: 12,
+    });
+  });
+
   test("dispatches reload, comment-add, comment-rm, and comment-clear commands", async () => {
     const { state, calls } = createFakeState();
     const requests: SessionDaemonRequest[] = [
diff --git a/src/session/broker/brokerServer.ts b/src/session/broker/brokerServer.ts
index b2b0406cc..f8b4f7c3c 100644
--- a/src/session/broker/brokerServer.ts
+++ b/src/session/broker/brokerServer.ts
@@ -279,6 +279,15 @@ function resolveNavigateCommandInput(
     };
   }
 
+  const hasAbsoluteTarget =
+    input.filePath !== undefined ||
+    input.hunkNumber !== undefined ||
+    input.side !== undefined ||
+    input.line !== undefined;
+  if (input.commentDirection !== undefined && hasAbsoluteTarget) {
+    throw new Error("navigate commentDirection cannot be combined with another navigation target.");
+  }
+
   if (
     !input.commentDirection &&
     input.hunkNumber === undefined &&
@@ -289,6 +298,14 @@ function resolveNavigateCommandInput(
     );
   }
 
+  // The live terminal cannot resolve a hunk or a line without the file that owns it.
+  if (
+    input.filePath === undefined &&
+    (input.hunkNumber !== undefined || input.line !== undefined)
+  ) {
+    throw new Error("navigate requires filePath for a hunk or line target.");
+  }
+
   // Exact coordinates take precedence so callers reveal the row rather than only its hunk.
   const hasExactLineTarget = input.side !== undefined && input.line !== undefined;
   return {