From e595663dc1e9d4663c4e766fd78b4124ca26827b Mon Sep 17 00:00:00 2001 From: Ashutosh <41143691+hack-monk@users.noreply.github.com> Date: Mon, 6 Jul 2026 15:23:29 -0400 Subject: [PATCH] fix: NaN filter bypass in LinkedIn CLI --jobage/--page/--limit flags (#35) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: silent zero output in salary converter and NaN filter bypass in LinkedIn CLI Bug #10 (convert_salary_excel.py): openpyxl ws[row_index] random access fails silently under read_only=True, leaving headers empty and producing no output. Fix: save the header row values during the existing iter_rows scan so ws[header_row] is never called. Bug #5 (.agents/skills/linkedin-search/cli/src/cli.ts): parseInt on --jobage/--page/--limit flags returns NaN on non-numeric input. NaN propagates silently — the jobage filter is dropped, page/limit are broken. Fix: validate each parsed int, exit 1 with a structured BAD_ARG error on NaN. Also adds: - tests/test_bug10_salary_converter.py: 10 scenarios, 31 assertions (all green) - tests/test_bug5_linkedin_cli.sh: 8 scenarios, 16 assertions (all green) - docs/bugfixes.md: root cause, impact, and fix explanation for both bugs, plus a note on the pre-existing detect_column_type "n" pattern issue Co-Authored-By: Claude Sonnet 4.6 * fix: address PR review — drop salary converter change, move test to bun - Drop tools/convert_salary_excel.py change (bug not reproducible on modern openpyxl; reviewer confirmed master works correctly) - Drop tests/test_bug10_salary_converter.py and top-level tests/ dir - Drop docs/bugfixes.md (analysis belongs in PR description, not repo) - Replace tests/test_bug5_linkedin_cli.sh with a proper bun test file at .agents/skills/linkedin-search/cli/tests/cli-flag-validation.test.ts following the jobindex-search/cli/tests/ convention (runCLI/parseJSON helpers, describe/test/expect, descriptive names) Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .agents/skills/linkedin-search/cli/src/cli.ts | 26 ++++++ .../cli/tests/cli-flag-validation.test.ts | 86 +++++++++++++++++++ .../linkedin-search/cli/tests/helpers.ts | 39 +++++++++ 3 files changed, 151 insertions(+) create mode 100644 .agents/skills/linkedin-search/cli/tests/cli-flag-validation.test.ts create mode 100644 .agents/skills/linkedin-search/cli/tests/helpers.ts diff --git a/.agents/skills/linkedin-search/cli/src/cli.ts b/.agents/skills/linkedin-search/cli/src/cli.ts index 15e0156..6e13fa5 100644 --- a/.agents/skills/linkedin-search/cli/src/cli.ts +++ b/.agents/skills/linkedin-search/cli/src/cli.ts @@ -83,6 +83,32 @@ async function main(): Promise { return 1 } const fmt = (flags.format as string) || "json" + + const parseIntFlag = (name: string, raw: string | boolean | string[]): number | null => { + const val = parseInt(raw as string, 10) + if (isNaN(val)) { + process.stderr.write(JSON.stringify({ error: `--${name} must be a number, got "${raw}"`, code: "BAD_ARG" }) + "\n") + return null + } + return val + } + + if (flags.jobage !== undefined) { + const v = parseIntFlag("jobage", flags.jobage) + if (v === null) return 1 + flags.jobage = String(v) + } + if (flags.page !== undefined) { + const v = parseIntFlag("page", flags.page) + if (v === null) return 1 + flags.page = String(v) + } + if (flags.limit !== undefined) { + const v = parseIntFlag("limit", flags.limit) + if (v === null) return 1 + flags.limit = String(v) + } + const opts: SearchOpts = { query: typeof flags.query === "string" ? flags.query : undefined, location, diff --git a/.agents/skills/linkedin-search/cli/tests/cli-flag-validation.test.ts b/.agents/skills/linkedin-search/cli/tests/cli-flag-validation.test.ts new file mode 100644 index 0000000..19bfce9 --- /dev/null +++ b/.agents/skills/linkedin-search/cli/tests/cli-flag-validation.test.ts @@ -0,0 +1,86 @@ +import { describe, test, expect } from "bun:test"; +import { runCLI } from "./helpers"; + +const LOCATION = "Copenhagen, Denmark"; + +function parsedStderr(stderr: string): { error?: string; code?: string } { + try { + return JSON.parse(stderr); + } catch { + return {}; + } +} + +describe("LinkedIn CLI flag validation", () => { + describe("--jobage NaN validation", () => { + test("non-numeric string exits 1 with BAD_ARG", async () => { + const result = await runCLI(["search", "-l", LOCATION, "--jobage", "foo"]); + expect(result.exitCode).not.toBe(0); + const err = parsedStderr(result.stderr); + expect(err.code).toBe("BAD_ARG"); + expect(err.error).toMatch(/jobage/); + }); + + test("boolean flag (no value) exits 1 with BAD_ARG", async () => { + const result = await runCLI(["search", "-l", LOCATION, "--jobage"]); + expect(result.exitCode).not.toBe(0); + expect(result.stderr).toBeTruthy(); + }); + + test("valid integer passes validation", async () => { + const result = await runCLI(["search", "-l", LOCATION, "--jobage", "7", "--limit", "1"]); + const err = parsedStderr(result.stderr); + expect(err.code).not.toBe("BAD_ARG"); + }); + + test("float string truncated to integer, no error", async () => { + // parseInt("7.5") = 7, which is valid + const result = await runCLI(["search", "-l", LOCATION, "--jobage", "7.5", "--limit", "1"]); + const err = parsedStderr(result.stderr); + expect(err.code).not.toBe("BAD_ARG"); + }); + + test("zero is accepted (falsy int should not be treated as missing)", async () => { + const result = await runCLI(["search", "-l", LOCATION, "--jobage", "0", "--limit", "1"]); + const err = parsedStderr(result.stderr); + expect(err.code).not.toBe("BAD_ARG"); + }); + }); + + describe("--page NaN validation", () => { + test("non-numeric string exits 1 with BAD_ARG", async () => { + const result = await runCLI(["search", "-l", LOCATION, "--page", "abc"]); + expect(result.exitCode).not.toBe(0); + const err = parsedStderr(result.stderr); + expect(err.code).toBe("BAD_ARG"); + expect(err.error).toMatch(/page/); + }); + }); + + describe("--limit NaN validation", () => { + test("non-numeric string exits 1 with BAD_ARG", async () => { + const result = await runCLI(["search", "-l", LOCATION, "--limit", "xyz"]); + expect(result.exitCode).not.toBe(0); + const err = parsedStderr(result.stderr); + expect(err.code).toBe("BAD_ARG"); + expect(err.error).toMatch(/limit/); + }); + }); + + describe("existing validations (regression)", () => { + test("missing --location exits 1 with NO_LOCATION", async () => { + const result = await runCLI(["search"]); + expect(result.exitCode).not.toBe(0); + const err = parsedStderr(result.stderr); + expect(err.code).toBe("NO_LOCATION"); + }); + + test("all valid flags produce no BAD_ARG", async () => { + const result = await runCLI([ + "search", "-l", LOCATION, "--jobage", "7", "--page", "1", "--limit", "5", + ]); + const err = parsedStderr(result.stderr); + expect(err.code).not.toBe("BAD_ARG"); + }); + }); +}); diff --git a/.agents/skills/linkedin-search/cli/tests/helpers.ts b/.agents/skills/linkedin-search/cli/tests/helpers.ts new file mode 100644 index 0000000..75b82b9 --- /dev/null +++ b/.agents/skills/linkedin-search/cli/tests/helpers.ts @@ -0,0 +1,39 @@ +import { join } from "path"; + +const CLI_PATH = join(import.meta.dir, "../src/cli.ts"); + +export interface CLIResult { + stdout: string; + stderr: string; + exitCode: number; +} + +export async function runCLI(args: string[]): Promise { + const proc = Bun.spawn(["bun", "run", CLI_PATH, ...args], { + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + + return { stdout: stdout.trim(), stderr: stderr.trim(), exitCode }; +} + +export function parseJSON(result: CLIResult): T { + if (result.exitCode !== 0) { + throw new Error( + `CLI exited with code ${result.exitCode}. stderr: ${result.stderr}` + ); + } + try { + return JSON.parse(result.stdout) as T; + } catch { + throw new Error( + `Failed to parse JSON. stdout: ${result.stdout}\nstderr: ${result.stderr}` + ); + } +}