From 0b02a63c655592ac2206ef7c680471fb728976bd Mon Sep 17 00:00:00 2001 From: Hako Date: Wed, 14 Oct 2026 22:10:39 +0900 Subject: [PATCH] refactor(server): simplify validated pane search options --- server/http.test.ts | 78 +++++++++++++++++++++++++++++++++++++++++++-- server/index.ts | 5 ++- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/server/http.test.ts b/server/http.test.ts index 716cb74cba..43b5a6c7c8 100644 --- a/server/http.test.ts +++ b/server/http.test.ts @@ -1,7 +1,12 @@ -import { describe, expect, it } from "bun:test"; +import { afterAll, afterEach, beforeAll, describe, expect, it, spyOn } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import * as herdr from "./herdr/client.ts"; import { HerdrError } from "./herdr/client.ts"; import { errorResponse } from "./http.ts"; +import { createServer } from "./index.ts"; describe("errorResponse", () => { it("answers a 500 with a fixed message and a correlation id, never the exception's own", async () => { @@ -25,4 +30,73 @@ describe("errorResponse", () => { expect(connect.error).toEqual({ code: "connect_failed", message: "herdr is not answering" }); expect(errorResponse(new HerdrError("pane_not_found", "no such pane")).status).toBe(404); }); -}); \ No newline at end of file +}); + +describe("pane find HTTP validation", () => { + const request = { pane_id: "w1:p1", query: "needle", direction: "forward" }; + const previous = { start: { row: 0, col: 0 }, end: { row: 0xffffffff, col: 0xffff } }; + let server: ReturnType; + let stateDir: string; + const find = spyOn(herdr, "paneFind").mockImplementation(async () => { + throw new HerdrError("stale_content", "search content changed"); + }); + const post = (patch: Record) => fetch(`http://127.0.0.1:${server.port}/api/pane/find`, { + method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ ...request, ...patch }), + }); + beforeAll(() => { + stateDir = mkdtempSync(join(tmpdir(), "herdr-find-http-")); + server = createServer({ port: 0, hostname: "127.0.0.1", stateDir, token: "", machines: false, sidecar: false }); + }); + afterEach(() => find.mockClear()); + afterAll(() => { + server.stop(); + find.mockRestore(); + rmSync(stateDir, { recursive: true, force: true }); + }); + + for (const patch of [ + {}, { jump: false }, { jump: true }, { content_revision: 0 }, + { previous, content_revision: Number.MAX_SAFE_INTEGER, jump: false }, + { previous: { ...previous, extra: "discard" }, content_revision: 0 }, + ]) { + it(`forwards validated optional fields for ${JSON.stringify(patch)}`, async () => { + // given + const expected = { ...request, ...patch, ...("previous" in patch ? { previous } : {}) }; + // when + const response = await post(patch); + // then + expect(find).toHaveBeenCalledTimes(1); + expect(find).toHaveBeenCalledWith(expected); + expect(response.status).toBe(409); + expect(await response.json()).toEqual({ error: { code: "stale_content", message: "search content changed" } }); + }); + } + + for (const [patch, code] of [ + [{ jump: null }, "invalid_jump"], [{ jump: 0 }, "invalid_jump"], [{ jump: "false" }, "invalid_jump"], + [{ content_revision: null }, "invalid_revision"], [{ content_revision: -1 }, "invalid_revision"], + [{ content_revision: 0.5 }, "invalid_revision"], [{ content_revision: Number.MAX_SAFE_INTEGER + 1 }, "invalid_revision"], + [{ previous: null, content_revision: 0 }, "invalid_previous"], + [{ previous: [], content_revision: 0 }, "invalid_previous"], + [{ previous: {}, content_revision: 0 }, "invalid_previous"], + [{ previous }, "invalid_previous"], [{ previous, content_revision: "0" }, "invalid_previous"], + ...[ + { row: -1, col: 0 }, { row: 0.5, col: 0 }, { row: 0x100000000, col: 0 }, + { row: 0, col: -1 }, { row: 0, col: 0.5 }, { row: 0, col: 0x10000 }, + { row: "0", col: 0 }, { row: 0, col: null }, + ].flatMap((point): [Record, string][] => [ + [{ previous: { ...previous, start: point }, content_revision: 0 }, "invalid_previous"], + [{ previous: { ...previous, end: point }, content_revision: 0 }, "invalid_previous"], + ]), + ] satisfies [Record, string][]) { + it(`rejects ${JSON.stringify(patch)} before calling herdr`, async () => { + // given: malformed optional navigation state + // when + const response = await post(patch); + // then + expect(response.status).toBe(400); + expect(await response.json()).toMatchObject({ error: { code } }); + expect(find).not.toHaveBeenCalled(); + }); + } +}); diff --git a/server/index.ts b/server/index.ts index 80cbed4bfc..93b3943554 100644 --- a/server/index.ts +++ b/server/index.ts @@ -124,7 +124,6 @@ const MAX_REPLAY_BYTES = 128 * 1024; const PLUGIN_PLATFORM = process.platform === "win32" ? "windows" : process.platform === "darwin" ? "macos" : "linux"; /** How long POST /api/plugin/action waits for the command herdr started to end before it answers `running`. */ const PLUGIN_ACTION_WAIT_MS = 5_000; -/** How far back in herdr's plugin command log a run is looked for. */ const livePaneIds = async (timeoutMs?: number): Promise => (await sessionSnapshot(undefined, timeoutMs)).panes.map((pane) => pane.pane_id); /** * herdr's refusal of an attach while a read of the same terminal is in progress; it asks @@ -2500,9 +2499,9 @@ export function createServer( try { return jsonResponse(await paneFind({ pane_id: payload.pane_id, query: payload.query, direction: payload.direction, - ...(typeof payload.jump === "boolean" ? { jump: payload.jump } : {}), + ...(payload.jump !== undefined ? { jump: payload.jump } : {}), ...(isJsonObject(previous) && point(previous.start) && point(previous.end) ? { previous: { start: previous.start, end: previous.end } } : {}), - ...(isCount(payload.content_revision) ? { content_revision: payload.content_revision } : {}), + ...(payload.content_revision !== undefined ? { content_revision: payload.content_revision } : {}), })); } catch (error) { if (error instanceof HerdrError && error.code === "stale_content") return conflictResponse(error.code, error.message);