From 407dc34a2c4951e4ea3b047eaccdf2e4081a139a Mon Sep 17 00:00:00 2001
From: Andrew Barnes <169967362+Bortlesboat@users.noreply.github.com>
Date: Thu, 15 Oct 2026 12:15:59 -0400
Subject: [PATCH] fix(freehire-search): reject empty facet filters before
 searching (#561)

* fix(freehire-search): reject empty facet filters before searching

* docs(changelog): record empty Freehire facet validation

* docs(changelog): avoid overlap with upstream additions
---
 .agents/skills/freehire-search/cli/src/cli.ts |  8 ++-
 .../cli/tests/cli-flag-validation.test.ts     | 55 +++++++++++++++++++
 CHANGELOG.md                                  |  5 ++
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/.agents/skills/freehire-search/cli/src/cli.ts b/.agents/skills/freehire-search/cli/src/cli.ts
index 16ec3022..d87c8133 100644
--- a/.agents/skills/freehire-search/cli/src/cli.ts
+++ b/.agents/skills/freehire-search/cli/src/cli.ts
@@ -39,7 +39,9 @@ function parseFlags(argv: string[]): Flags {
     // --facet repeats; collect into an array. Everything else is last-wins.
     if (key === "facet") {
       const acc = Array.isArray(flags.facet) ? flags.facet : []
-      if (typeof value === "string") acc.push(value)
+      // Preserve a bare occurrence so validation can reject it instead of
+      // silently dropping the requested filter, even after a valid --facet.
+      acc.push(typeof value === "string" ? value : "")
       flags.facet = acc
     } else {
       flags[key] = value
@@ -207,6 +209,10 @@ async function main(): Promise<number> {
       }
       const key = kv.slice(0, eq)
       const vals = commaList(kv.slice(eq + 1))
+      if (vals.length === 0) {
+        process.stderr.write(JSON.stringify({ error: `invalid --facet "${kv}", want key=value with at least one value`, code: "BAD_ARG" }) + "\n")
+        return 1
+      }
       facets[key] = (facets[key] ?? []).concat(vals)
     }
 
diff --git a/.agents/skills/freehire-search/cli/tests/cli-flag-validation.test.ts b/.agents/skills/freehire-search/cli/tests/cli-flag-validation.test.ts
index 372f94eb..a84e8eaa 100644
--- a/.agents/skills/freehire-search/cli/tests/cli-flag-validation.test.ts
+++ b/.agents/skills/freehire-search/cli/tests/cli-flag-validation.test.ts
@@ -68,6 +68,61 @@ describe("freehire CLI flag validation", () => {
   });
 
   describe("--facet validation", () => {
+    for (const args of [
+      ["--facet"],
+      ["--facet", "--limit", "1"],
+      ["--facet", "salary_min="],
+      ["--facet", "salary_min= , "],
+      ["--facet", "countries=US", "--facet"],
+    ]) {
+      test(`${args.join(" ")} rejects an empty filter before requesting jobs`, async () => {
+        const requests: string[] = [];
+        const server = Bun.serve({
+          hostname: "127.0.0.1",
+          port: 0,
+          fetch(request) {
+            requests.push(request.url);
+            return Response.json({ data: [], meta: { total: 0 } });
+          },
+        });
+        try {
+          const result = await runCLI(["search", ...args], { FREEHIRE_API_URL: server.url.toString() });
+          expect(result.exitCode).toBe(1);
+          expect(result.stdout).toBe("");
+          expect(parsedStderr(result.stderr).code).toBe("BAD_ARG");
+          expect(parsedStderr(result.stderr).error).toContain("--facet");
+          expect(requests).toEqual([]);
+        } finally {
+          server.stop(true);
+        }
+      });
+    }
+
+    test("repeated facets and comma-separated values reach the API", async () => {
+      const requests: URL[] = [];
+      const server = Bun.serve({
+        hostname: "127.0.0.1",
+        port: 0,
+        fetch(request) {
+          requests.push(new URL(request.url));
+          return Response.json({ data: [], meta: { total: 0 } });
+        },
+      });
+      try {
+        const result = await runCLI([
+          "search", "--facet", "countries=US, CA", "--facet", "countries=GB",
+          "--facet", "salary_min=100000", "--remote",
+        ], { FREEHIRE_API_URL: server.url.toString() });
+        expect(result.exitCode).toBe(0);
+        expect(requests).toHaveLength(1);
+        expect(requests[0].searchParams.getAll("countries")).toEqual(["US", "CA", "GB"]);
+        expect(requests[0].searchParams.get("salary_min")).toBe("100000");
+        expect(requests[0].searchParams.get("work_mode")).toBe("remote");
+      } finally {
+        server.stop(true);
+      }
+    });
+
     test("a facet without '=' exits 1 with BAD_ARG", async () => {
       const result = await runCLI(["search", "--facet", "novalue"]);
       expect(result.exitCode).not.toBe(0);
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 521158ab..e341a0ed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -156,6 +156,11 @@ per-file diff commands.
   derive both checks from the shipped files, so a renamed heading fails until `/reset`
   follows it; both fail on master.
 
+- **`freehire-search` rejects empty `--facet` filters before requesting jobs** (#561) -
+  a bare flag (including one after a valid repeated facet), `salary_min=`, or a
+  comma-separated value list containing only whitespace now returns `BAD_ARG` and
+  exits 1 instead of silently dropping the filter and running a broader search.
+
 - **The `/apply` host-check test derives its installed-portal hosts from the skills tree
   instead of a hand-copied set** (#497, `tests/test_apply_host_check.py`) - `SHIPPED_PORTAL_HOSTS`
   was six names typed into the test and nothing tied it to anything: a portal added by