mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-09-17 00:26:26 +00:00
fix: NaN filter bypass in LinkedIn CLI --jobage/--page/--limit flags (#35)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
79b153764d
commit
e595663dc1
@@ -83,6 +83,32 @@ async function main(): Promise<number> {
|
|||||||
return 1
|
return 1
|
||||||
}
|
}
|
||||||
const fmt = (flags.format as string) || "json"
|
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 = {
|
const opts: SearchOpts = {
|
||||||
query: typeof flags.query === "string" ? flags.query : undefined,
|
query: typeof flags.query === "string" ? flags.query : undefined,
|
||||||
location,
|
location,
|
||||||
|
|||||||
@@ -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");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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<CLIResult> {
|
||||||
|
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<T = unknown>(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}`
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user