fix(session): explain selector misses instead of protocol-validation-failed

.changeset/quiet-worktrees-explain.md

@@ -0,0 +1,5 @@
+---
+"hunkdiff": patch
+---
+
+`hunk session` commands that select a window with `--repo` or `--session-path` now say which path matched no live session, instead of failing with `protocol-validation-failed` (for example, when run from a different git worktree than the Hunk window). The session daemon produces this message, so it appears once a daemon from this release is running; an older daemon keeps answering with `protocol-validation-failed` until it restarts.

packages/hunk/src/session/broker/brokerServer.test.ts

@@ -106,6 +106,7 @@ async function authenticatedFetch(
   port: number,
   path: string,
   init: SessionBrokerSignedRequestInit = {},
+  options: { targetSpecific?: boolean } = {},
 ) {
   const credentials = await loadOrCreateHunkSessionBrokerCredentials();
   const caller = new SessionBrokerCallerClient({
@@ -120,7 +121,7 @@ async function authenticatedFetch(
       ? ((JSON.parse(init.body) as { action?: string }).action ?? "")
       : "";
   return caller.request(path, init, {
-    targetSpecific: path === "/session-api" && action !== "list",
+    targetSpecific: options.targetSpecific ?? (path === "/session-api" && action !== "list"),
   });
 }
 
@@ -939,6 +940,82 @@ describe("Hunk session daemon server", () => {
     }
   });
 
+  test("reports why a repo selector matched no session instead of a redacted failure", async () => {
+    const port = await reserveLoopbackPort();
+    process.env.HUNK_MCP_HOST = "127.0.0.1";
+    process.env.HUNK_MCP_PORT = String(port);
+
+    const original = SessionBrokerState.prototype.dispatchCommand;
+    const dispatched: string[] = [];
+    SessionBrokerState.prototype.dispatchCommand = (({ command }: { command: string }) => {
+      dispatched.push(command);
+      return Promise.reject(new Error("A selector miss must not dispatch."));
+    }) as SessionBrokerState["dispatchCommand"];
+
+    const server = await serveSessionBrokerDaemon();
+    let socket: WebSocket | undefined;
+
+    try {
+      // The live window is showing /repo; the caller is in a different checkout.
+      socket = await openRegisteredSession(port);
+      for (const request of [
+        { action: "get", selector: { repoRoot: "/elsewhere" } },
+        {
+          action: "reload",
+          selector: { repoRoot: "/elsewhere" },
+          nextInput: { kind: "vcs", staged: false, options: {} },
+        },
+      ]) {
+        const response = await authenticatedFetch(port, "/session-api", {
+          method: "POST",
+          headers: { "content-type": "application/json" },
+          body: JSON.stringify(request),
+        });
+
+        expect(response.status).toBe(400);
+        await expect(response.json()).resolves.toEqual({
+          error: "No active session matches repoRoot /elsewhere.",
+        });
+      }
+      expect(dispatched).toEqual([]);
+    } finally {
+      SessionBrokerState.prototype.dispatchCommand = original;
+      socket?.close();
+      server.stop(true);
+    }
+  });
+
+  test("keeps malformed session API bodies redacted", async () => {
+    const port = await reserveLoopbackPort();
+    process.env.HUNK_MCP_HOST = "127.0.0.1";
+    process.env.HUNK_MCP_PORT = String(port);
+
+    const server = await serveSessionBrokerDaemon();
+
+    try {
+      const response = await authenticatedFetch(
+        port,
+        "/session-api",
+        {
+          method: "POST",
+          headers: { "content-type": "application/json" },
+          body: JSON.stringify({
+            action: "reload",
+            selector: { repoRoot: "/repo" },
+            nextInput: { kind: "vcs", staged: false, options: { notAnOption: true } },
+          }),
+        },
+        // An unparseable body has no trustworthy target, so the daemon signs it untargeted.
+        { targetSpecific: false },
+      );
+
+      expect(response.status).toBe(400);
+      await expect(response.json()).resolves.toEqual({ error: "protocol-validation-failed" });
+    } finally {
+      server.stop(true);
+    }
+  });
+
   test("serves review notes through the session API", async () => {
     const port = await reserveLoopbackPort();
     process.env.HUNK_MCP_HOST = "127.0.0.1";

packages/hunk/src/session/broker/brokerServer.ts

@@ -1,6 +1,7 @@
 import {
   SESSION_BROKER_ADMIN_SCOPE_VERSION,
   SessionBrokerAuthenticator,
+  SessionBrokerTargetResolutionError,
   createSessionBrokerDaemon,
   type SessionBrokerAuthenticatedControlFacts,
   type SessionBrokerController,
@@ -357,14 +358,32 @@ function resolveNavigateCommandInput(
   };
 }
 
-/** Map each Hunk action to the generic operation and exact producer command scope it requires. */
+/**
+ * Map each Hunk action to the generic operation and exact producer command scope it requires.
+ *
+ * A `--repo` or `--session-path` selector names its session only after a lookup, so the lookup
+ * runs before authorization. When it matches no single session, the lookup error becomes a
+ * target-resolution failure that the daemon discloses only to callers allowed to `list`: the error
+ * names nothing a `list` would not, so the Hunk CLI learns why its selector missed, and a
+ * session-scoped caller still learns nothing. Malformed bodies keep throwing plain errors so the
+ * daemon answers with its redacted protocol failure.
+ */
 function sessionApiAuthorizationFacts(
   state: HunkSessionBrokerState,
   bytes: Uint8Array,
 ): SessionBrokerAuthenticatedControlFacts {
   const input = parseJsonRequestBytes(bytes);
   if (input.action === "list") return { operation: "list", targetSpecific: false };
-  const sessionId = input.selector.sessionId ?? state.getSession(input.selector).sessionId;
+  let sessionId = input.selector.sessionId;
+  if (sessionId === undefined) {
+    try {
+      sessionId = state.getSession(input.selector).sessionId;
+    } catch (error) {
+      throw new SessionBrokerTargetResolutionError(
+        error instanceof Error ? error.message : "No active session matches.",
+      );
+    }
+  }
   if (["get", "context", "review", "comment-list"].includes(input.action)) {
     return { operation: "get", sessionId, targetSpecific: true };
   }

packages/session-broker/src/daemon.test.ts

@@ -12,7 +12,7 @@ import {
 } from "@hunk/session-broker-core";
 import { SessionBroker } from "./broker";
 import { SESSION_BROKER_ADMIN_STOP_CLOSE_REASON } from "./admin";
-import { createSessionBrokerDaemon } from "./daemon";
+import { SessionBrokerTargetResolutionError, createSessionBrokerDaemon } from "./daemon";
 import { createSessionBrokerProtocolParsers } from "./protocolParsers";
 import type {
   AuthenticatedCallerRequest,
@@ -1402,6 +1402,95 @@ describe("session broker daemon", () => {
     daemon.shutdown();
   });
 
+  test("discloses target resolution failures only to callers allowed to list, without handling", async () => {
+    const audited: { operation: string; decision: string; outcome: string }[] = [];
+    const caller = (principal: Partial<CallerPrincipal>) =>
+      createSessionBrokerDaemon({
+        broker: createBroker(),
+        exposeHttpApi: true,
+        appId: "session-broker",
+        appRevision: 1,
+        callerAuthenticator: {
+          authenticate: async () =>
+            authenticatedRequest({
+              kind: "caller",
+              appId: "session-broker",
+              principalId: "test-caller",
+              keyId: "test-key",
+              grantId: "test-grant",
+              operations: ["list", "get", "dispatch"],
+              commands: [],
+              ...principal,
+            }),
+        },
+        authorizer: async () => true,
+        audit: ({ operation, decision, outcome }) => {
+          audited.push({ operation, decision, outcome });
+        },
+      });
+    let handled = 0;
+    const control = {
+      resolve: () => {
+        throw new SessionBrokerTargetResolutionError("No active session matches repoRoot /x.");
+      },
+      handle: () => {
+        handled += 1;
+        return { body: { ok: true } };
+      },
+    };
+    const request = () => new Request("http://broker.test/custom", { method: "POST", body: "{}" });
+
+    const lister = caller({});
+    const disclosed = await lister.handleAuthenticatedControl(request(), control);
+    expect(disclosed.status).toBe(400);
+    const disclosedEnvelope = (await disclosed.json()) as {
+      body: unknown;
+      authentication: { appContract?: unknown };
+    };
+    expect(disclosedEnvelope.body).toEqual({ error: "No active session matches repoRoot /x." });
+    // The caller signed a targeted request, so the answer must carry the app contract.
+    expect(disclosedEnvelope.authentication.appContract).toBeDefined();
+    lister.shutdown();
+
+    // A session-scoped caller can never list, so it learns nothing about other sessions.
+    const scoped = caller({ sessionId: "session-1" });
+    const denied = await scoped.handleAuthenticatedControl(request(), control);
+    expect(denied.status).toBe(403);
+    expect(await authenticatedBody(denied)).toEqual({ error: "authorization-denied" });
+    scoped.shutdown();
+
+    const unlisted = caller({ operations: ["get", "dispatch"] });
+    expect((await unlisted.handleAuthenticatedControl(request(), control)).status).toBe(403);
+    unlisted.shutdown();
+
+    expect(handled).toBe(0);
+    expect(audited).toEqual([
+      { operation: "list", decision: "allow", outcome: "target-resolution-failed" },
+      { operation: "list", decision: "deny", outcome: "authorization-failed" },
+      { operation: "list", decision: "deny", outcome: "authorization-failed" },
+    ]);
+  });
+
+  test("keeps other resolve failures redacted", async () => {
+    const daemon = createSessionBrokerDaemon({
+      broker: createBroker(),
+      exposeHttpApi: true,
+      ...authenticatedHttpApi,
+    });
+    const response = await daemon.handleAuthenticatedControl(
+      new Request("http://broker.test/custom", { method: "POST", body: "{}" }),
+      {
+        resolve: () => {
+          throw new Error("private parser detail");
+        },
+        handle: () => ({ body: { ok: true } }),
+      },
+    );
+    expect(response.status).toBe(400);
+    expect(await authenticatedBody(response)).toEqual({ error: "protocol-validation-failed" });
+    daemon.shutdown();
+  });
+
   test("rejects daemon state limits that were not configured on its broker controller", () => {
     expect(() =>
       createSessionBrokerDaemon({

packages/session-broker/src/daemon.ts

@@ -92,6 +92,31 @@ export interface SessionBrokerAuthenticatedControlResult {
   readonly status?: number;
 }
 
+/**
+ * Reports that an authenticated control's selector resolved to no single session.
+ *
+ * Thrown from `handleAuthenticatedControl`'s `resolve` once the body has parsed. The daemon never
+ * runs `handle` for it; it answers with `message` only when the caller holds
+ * `disclosureOperation`, whose grant must already reveal everything the message names (with
+ * `list`, the matching session IDs and titles). Every other `resolve` error stays a redacted
+ * protocol failure.
+ */
+export class SessionBrokerTargetResolutionError extends Error {
+  readonly disclosureOperation: CallerOperation;
+  readonly targetSpecific: boolean;
+
+  constructor(
+    message: string,
+    options: { disclosureOperation?: CallerOperation; targetSpecific?: boolean } = {},
+  ) {
+    super(message);
+    this.name = "SessionBrokerTargetResolutionError";
+    this.disclosureOperation = options.disclosureOperation ?? "list";
+    // A selector that failed to resolve was aimed at a session, so the caller signed it targeted.
+    this.targetSpecific = options.targetSpecific ?? true;
+  }
+}
+
 /**
  * Configure the revision-tolerant admin scope (`status` and `stop`).
  *
@@ -1016,7 +1041,10 @@ export class SessionBrokerDaemon<
       let facts: SessionBrokerAuthenticatedControlFacts;
       try {
         facts = options.resolve(body);
-      } catch {
+      } catch (error) {
+        if (error instanceof SessionBrokerTargetResolutionError) {
+          return this.answerTargetResolutionFailure(request, authenticated, error);
+        }
         let targetSpecific = false;
         try {
           targetSpecific = options.resolveFailureTargetSpecific?.(body) ?? false;
@@ -1059,6 +1087,41 @@ export class SessionBrokerDaemon<
     });
   }
 
+  /**
+   * Answer a control whose selector resolved to no single session, without running its action.
+   *
+   * Authorizes only the disclosure, audited under its own outcome so a missed `reload` never
+   * reads as a successful `list`.
+   */
+  private async answerTargetResolutionFailure(
+    request: Request,
+    authenticated: AuthenticatedCallerRequest,
+    failure: SessionBrokerTargetResolutionError,
+  ): Promise<Response> {
+    const allowed = await this.authorize(
+      request,
+      authenticated,
+      { operation: failure.disclosureOperation },
+      "target-resolution-failed",
+    );
+    if (!allowed) {
+      return this.authenticatedResponse(
+        authenticated,
+        { error: "authorization-denied" },
+        403,
+        failure.targetSpecific,
+      );
+    }
+    const inactive = this.rejectInactiveRequest(authenticated);
+    if (inactive) return inactive;
+    return this.authenticatedResponse(
+      authenticated,
+      { error: failure.message },
+      400,
+      failure.targetSpecific,
+    );
+  }
+
   private async authenticateRequest(
     request: Request,
     body: Uint8Array,
@@ -1121,6 +1184,7 @@ export class SessionBrokerDaemon<
       command?: string;
       commandVersion?: number;
     },
+    allowedOutcome: "authenticated" | "target-resolution-failed" = "authenticated",
   ): Promise<boolean> {
     const principal: CallerPrincipal = authenticated.principal;
     const allowedByGrant = callerPrincipalAllows(principal, {
@@ -1151,7 +1215,7 @@ export class SessionBrokerDaemon<
       ...(facts.commandVersion === undefined ? {} : { commandVersion: facts.commandVersion }),
       requestId: authenticated.requestId,
       decision: allowedByApp ? "allow" : "deny",
-      outcome: allowedByApp ? "authenticated" : "authorization-failed",
+      outcome: allowedByApp ? allowedOutcome : "authorization-failed",
       timestamp: Date.now(),
     });
     return allowedByApp;

packages/session-broker/src/types.ts

@@ -133,7 +133,12 @@ export interface SessionBrokerAuditEvent {
   readonly commandVersion?: number;
   readonly requestId?: string;
   readonly decision: "allow" | "deny";
-  readonly outcome: "authenticated" | "authentication-failed" | "authorization-failed";
+  readonly outcome:
+    | "authenticated"
+    | "authentication-failed"
+    | "authorization-failed"
+    /** The caller may learn why its selector matched no single session; no action ran. */
+    | "target-resolution-failed";
   readonly timestamp: number;
 }