From 69bfaa291f22e795d7a4318c3ba202e55644762d Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Wed, 9 Sep 2026 00:03:37 +0000 Subject: [PATCH 1/4] refactor(plugin): migrate rank shard helpers to TypeScript --- .../codex-security/mcp-app/helpers-main.ts | 9 +- .../mcp-app/src/helpers/deep-review-input.ts | 241 +------ .../mcp-app/src/helpers/rank-shards.ts | 295 +++++++++ .../mcp-app/src/helpers/rank-worklists.ts | 232 +++++++ plugins/codex-security/native/README.md | 2 +- .../native/examples/windows-wide-launcher.rs | 104 ++- .../scripts/generate_rank_input.py | 113 ---- .../tests/test_generate_rank_input.py | 275 +------- sdk/typescript/tests-ts/rank-shards.test.ts | 613 ++++++++++++++++++ 9 files changed, 1284 insertions(+), 600 deletions(-) create mode 100644 plugins/codex-security/mcp-app/src/helpers/rank-shards.ts create mode 100644 plugins/codex-security/mcp-app/src/helpers/rank-worklists.ts create mode 100644 sdk/typescript/tests-ts/rank-shards.test.ts diff --git a/plugins/codex-security/mcp-app/helpers-main.ts b/plugins/codex-security/mcp-app/helpers-main.ts index 63920a951..3beb90680 100644 --- a/plugins/codex-security/mcp-app/helpers-main.ts +++ b/plugins/codex-security/mcp-app/helpers-main.ts @@ -5,6 +5,7 @@ import { windowsBinding } from "./src/native"; import { normalizeCandidatesCommand } from "./src/helpers/normalize-candidates"; import { validatePatchRiskAssessmentCommand } from "./src/helpers/validate-patch-risk-assessment"; import { deepReviewInputCommand } from "./src/helpers/deep-review-input"; +import { rankShardsCommand } from "./src/helpers/rank-shards"; let commandLine = process.argv.slice(2); if (process.platform === "win32") { @@ -41,9 +42,15 @@ if (command === "resolve-security-md") { command === "select-deep-review-input" ) { process.exitCode = deepReviewInputCommand(command, args, posixHome); +} else if ( + command === "make-rank-shards" || + command === "validate-rank-shard" || + command === "merge-rank-outputs" +) { + process.exitCode = rankShardsCommand(command, args, posixHome); } else { console.error( - "Usage: launch_codex_security_mcp[.cmd] --helper [options]", + "Usage: launch_codex_security_mcp[.cmd] --helper [options]", ); process.exitCode = 2; } diff --git a/plugins/codex-security/mcp-app/src/helpers/deep-review-input.ts b/plugins/codex-security/mcp-app/src/helpers/deep-review-input.ts index 7466b5844..d67a8c93f 100644 --- a/plugins/codex-security/mcp-app/src/helpers/deep-review-input.ts +++ b/plugins/codex-security/mcp-app/src/helpers/deep-review-input.ts @@ -1,220 +1,15 @@ -import { decodeUtf8 } from "./utf8"; -import { dirname } from "node:path"; -import { exists, mkdir, readFile, writeFile } from "./helper-files"; -import { encodePosixPath } from "./posix-path"; -import { JsonSyntaxError, object, parseJson, pythonRepr } from "./python-json"; -import { expandHome, parsedPath } from "./resolve-security-md"; +import { + ArgumentError, + argumentsFor, + compare, + loadRankRows, + print, + requireUniquePaths, + worklistPath, + writeRankRows, +} from "./rank-worklists"; type Command = "copy-deep-review-input" | "select-deep-review-input"; -interface RankRow { - path: string; - area: string; - score?: bigint; - include?: boolean; -} -const trim = (value: string) => - value.replace( - /^[\p{White_Space}\u001c-\u001f]+|[\p{White_Space}\u001c-\u001f]+$/gu, - "", - ); - -function compare(left: string, right: string): number { - const a = Array.from(left, (character) => character.codePointAt(0)!); - const b = Array.from(right, (character) => character.codePointAt(0)!); - for (let index = 0; index < Math.min(a.length, b.length); index++) { - if (a[index] !== b[index]) return a[index]! - b[index]!; - } - return a.length - b.length; -} - -function loadRows(path: string, selection: boolean): RankRow[] { - const label = selection ? "Rank output" : "Rank input"; - if (!exists(path)) throw new Error(`${label} missing: ${path}`); - const contents = decodeUtf8(readFile(path)); - const lines = contents === "" ? [] : contents.split(/\r\n|[\r\n]/u); - if (lines.at(-1) === "") lines.pop(); - const fields = selection - ? ["path", "area", "score", "include", "reason"] - : ["path", "area", "preview"]; - const rows = lines.map((line, index) => { - const fail = (message: string): never => { - throw new Error(`${path}:${index + 1}: ${message}`); - }; - if (trim(line) === "") fail("blank JSONL rows are not allowed"); - let row: unknown; - try { - row = parseJson(line); - } catch (error) { - if (error instanceof JsonSyntaxError) - fail( - `invalid JSON: ${error.message.replace(/: line \d+ column \d+ \(char \d+\)$/u, "")}`, - ); - throw error; - } - if (!object(row)) return fail("expected a JSON object"); - const missing = fields - .filter((field) => !Object.hasOwn(row, field)) - .sort(compare); - const unexpected = Object.keys(row) - .filter((field) => !fields.includes(field)) - .sort(compare); - const details: string[] = []; - if (missing.length) details.push(`missing fields ${pythonRepr(missing)}`); - if (unexpected.length) - details.push(`unexpected fields ${pythonRepr(unexpected)}`); - if (details.length) fail(details.join("; ")); - for (const field of selection - ? ["path", "area"] - : ["path", "area", "preview"]) { - if ( - typeof row[field] !== "string" || - (field === "path" && trim(row[field]) === "") - ) - fail( - `${field} must be ${field === "path" ? "a non-empty string" : "a string"}`, - ); - } - if (selection) { - if (typeof row.score !== "bigint") - fail("score must be an integer from 1 through 10"); - if ((row.score as bigint) < 1n || (row.score as bigint) > 10n) - fail("score must be from 1 through 10"); - if (typeof row.include !== "boolean") fail("include must be a boolean"); - if (typeof row.reason !== "string" || trim(row.reason) === "") - fail("reason must be a non-empty string"); - } - return row as unknown as RankRow; - }); - const seen = new Set(), - duplicates = new Set(); - for (const row of rows) { - if (seen.has(row.path)) duplicates.add(row.path); - seen.add(row.path); - } - if (duplicates.size) - throw new Error( - `${label} contains duplicate paths: ${pythonRepr([...duplicates].sort(compare))}`, - ); - return rows; -} - -function writeRows(output: string, rows: RankRow[]): void { - mkdir(dirname(output)); - function* contents(): Iterable { - for (const row of rows) { - const json = JSON.stringify({ path: row.path, area: row.area }).replace( - /[\u007f-\uffff]/g, - (character) => - `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, - ); - yield Buffer.from(json + (process.platform === "win32" ? "\r\n" : "\n")); - } - } - writeFile(output, contents()); -} - -class ArgumentError extends Error {} -function integer(value: string): bigint { - const text = value.replace(/^\p{White_Space}+|\p{White_Space}+$/gu, ""); - if (!/^[+-]?\p{Decimal_Number}+(?:_\p{Decimal_Number}+)*$/u.test(text)) - throw new ArgumentError( - `argument --top-percent: invalid int value: ${pythonRepr(value)}`, - ); - return BigInt( - Array.from(text.replaceAll("_", ""), (character) => { - if (!/\p{Decimal_Number}/u.test(character)) return character; - const point = character.codePointAt(0)!; - let start = point; - while (/\p{Decimal_Number}/u.test(String.fromCodePoint(start - 1))) - start--; - return String((point - start) % 10); - }).join(""), - ); -} - -function argumentsFor( - args: string[], - selection: boolean, -): Record { - const input = selection ? "rank-output" : "rank-input"; - const names = [input, "out", "help", ...(selection ? ["top-percent"] : [])]; - const values: Record = {}; - const extra: string[] = []; - const looksOptional = (arg: string) => - arg.startsWith("-") && - arg !== "-" && - !arg.includes(" ") && - !/^-(?:\p{Decimal_Number}+|\p{Decimal_Number}*\.\p{Decimal_Number}+)\n?$/u.test( - arg, - ); - for (let index = 0; index < args.length; index++) { - const arg = args[index]!; - if (arg === "--") { - extra.push(...args.slice(index)); - break; - } - const equals = arg.indexOf("="); - const option = equals === -1 ? arg : arg.slice(0, equals); - const matches = option.startsWith("--") - ? names.filter((name) => `--${name}`.startsWith(option)) - : []; - const name = - names.find((name) => option === `--${name}`) ?? - (arg.startsWith("-h") - ? "help" - : matches.length === 1 - ? matches[0] - : undefined); - if (matches.length > 1 && !name) - throw new ArgumentError( - `ambiguous option: ${arg} could match ${matches.map((name) => `--${name}`).join(", ")}`, - ); - if (!name) { - extra.push(arg); - continue; - } - if (name === "help") { - if (equals !== -1) - throw new ArgumentError( - `argument -h/--help: ignored explicit argument ${pythonRepr(arg.slice(equals + 1))}`, - ); - return { help: true }; - } - let value: string; - if (equals !== -1) value = arg.slice(equals + 1); - else { - if (index + 1 === args.length || looksOptional(args[index + 1]!)) - throw new ArgumentError(`argument --${name}: expected one argument`); - value = args[++index]!; - } - values[name] = name === "top-percent" ? integer(value) : value; - } - const missing = [input, "out"].filter((name) => values[name] === undefined); - if (missing.length) - throw new ArgumentError( - `the following arguments are required: ${missing.map((name) => `--${name}`).join(", ")}`, - ); - if (extra.length) - throw new ArgumentError(`unrecognized arguments: ${extra.join(" ")}`); - return values; -} - -function print(message: string, stderr = false): void { - let text = message + "\n"; - if (stderr) - text = text.replace( - /[\ud800-\udfff]/gu, - (character) => - `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, - ); - (stderr ? process.stderr : process.stdout).write( - process.platform === "win32" - ? text.replaceAll("\n", "\r\n") - : stderr - ? text - : encodePosixPath(text), - ); -} export function deepReviewInputCommand( command: Command, @@ -225,7 +20,11 @@ export function deepReviewInputCommand( const input = selection ? "rank-output" : "rank-input"; const usage = `usage: launch_codex_security_mcp[.cmd] --helper ${command} [-h] --${input} PATH --out PATH${selection ? " [--top-percent INT]" : ""}`; try { - const values = argumentsFor(args, selection); + const values = argumentsFor( + args, + [input, "out"], + selection ? ["top-percent"] : [], + ); if (values.help) { print( `${usage}\n\nCreate deep_review_input.jsonl from ${selection ? "worker-produced rank_output.jsonl" : "rank_input.jsonl"}.\n\noptions:\n -h, --help show this help message and exit\n --${input} PATH ${selection ? "Worker ranking output" : "Deterministic rank input"} JSONL.\n --out PATH Output deep_review_input.jsonl path.${selection ? "\n --top-percent INT Percent of included files to keep for deep review. Defaults to 100." : ""}`, @@ -233,8 +32,9 @@ export function deepReviewInputCommand( return 0; } const path = (name: string) => - parsedPath(expandHome(parsedPath(values[name] as string), posixHome)); - const rows = loadRows(path(input), selection); + worklistPath(values[name] as string, posixHome); + const rows = loadRankRows(path(input), selection); + requireUniquePaths(rows, selection ? "Rank output" : "Rank input"); let selected = rows, total = rows.length; if (selection) { @@ -258,7 +58,10 @@ export function deepReviewInputCommand( selected = base.slice(0, keep); } const output = path("out"); - writeRows(output, selected); + writeRankRows( + output, + selected.map(({ path, area }) => ({ path, area })), + ); const message = selection ? `Selected ${selected.length} of ${total} rows into ${output}` : `Copied ${selected.length} rows into ${output}`; diff --git a/plugins/codex-security/mcp-app/src/helpers/rank-shards.ts b/plugins/codex-security/mcp-app/src/helpers/rank-shards.ts new file mode 100644 index 000000000..34091c5d9 --- /dev/null +++ b/plugins/codex-security/mcp-app/src/helpers/rank-shards.ts @@ -0,0 +1,295 @@ +import { statSync } from "node:fs"; +import { basename, sep } from "node:path"; +import { unixBinding, windowsBinding } from "../native"; +import { + pathText, + widePath, + windowsFileSystem, + windowsJoin, +} from "../../../native/windows-files.mjs"; +import { mkdir } from "./helper-files"; +import { decodePosixBytes, encodePosixPath } from "./posix-path"; +import { pythonRepr } from "./python-json"; +import { parsedPath } from "./resolve-security-md"; +import { + ArgumentError, + argumentsFor, + compare, + loadRankRows, + print, + requireUniquePaths, + worklistPath, + writeRankRows, + type RankRow, +} from "./rank-worklists"; + +type Command = + | "make-rank-shards" + | "validate-rank-shard" + | "merge-rank-outputs"; + +export function childPath(directory: string, name: string): string { + return parsedPath( + process.platform === "win32" + ? windowsJoin(directory, name) + : directory + sep + name, + ); +} + +function shardNames(directory: string, kind: "input" | "output"): string[] { + const matches = (name: string) => { + // Python's Unicode case-insensitive globbing includes these ASCII equivalents. + const matched = + process.platform === "win32" + ? name.replace(/[İı]/gu, "i").replace(/ſ/gu, "s").toLowerCase() + : name; + return ( + matched.startsWith("rank-shard-") && matched.endsWith(`.${kind}.jsonl`) + ); + }; + try { + if (process.platform === "win32") + return windowsFileSystem(windowsBinding()) + .entriesWithTypes(widePath(directory)) + .map((entry) => pathText(entry.name)) + .filter(matches); + const { errno, value } = unixBinding().directoryEntries( + encodePosixPath(directory), + false, + ); + return errno + ? [] + : value.map((entry) => decodePosixBytes(entry.name)).filter(matches); + } catch (error) { + // pathlib glob ignores directory enumeration OSErrors. + if (!(error instanceof Error) || !("errno" in error || "winerror" in error)) + throw error; + return []; + } +} + +function discoverInputShards(directory: string): string[] { + let isDirectory = false; + try { + isDirectory = ( + process.platform === "win32" + ? windowsFileSystem(windowsBinding()).stat(widePath(directory)) + : statSync(encodePosixPath(directory)) + ).isDirectory(); + } catch (error) { + const { code, winerror } = error as NodeJS.ErrnoException & { + winerror?: number; + }; + if ( + !["ENOENT", "ENOTDIR", "EBADF", "ELOOP"].includes(code ?? "") && + winerror !== 21 && + winerror !== 123 + ) + throw error; + } + if (!isDirectory) + throw new Error(`Rank shard directory missing: ${directory}`); + const numbered = shardNames(directory, "input").map((name) => { + const match = /^rank-shard-([0-9]{4,})\.input\.jsonl$/u.exec(name); + if (!match || match[0] !== name) + throw new Error(`Rank input shard has invalid name: ${name}`); + return { name, number: BigInt(match[1]!) }; + }); + numbered.sort((left, right) => + left.number === right.number + ? compare(left.name, right.name) + : left.number < right.number + ? -1 + : 1, + ); + const names = numbered.map(({ name }) => name); + const expected = names.map( + (_, index) => + `rank-shard-${String(index + 1).padStart(4, "0")}.input.jsonl`, + ); + if (names.some((name, index) => name !== expected[index])) + throw new Error( + `Rank input shards must use contiguous canonical names; expected=${pythonRepr(expected)}; actual=${pythonRepr(names)}`, + ); + return names.map((name) => childPath(directory, name)); +} + +function validateShard(input: string, output: string): [RankRow[], RankRow[]] { + const inputs = loadRankRows(input, false, "Rank input shard"); + requireUniquePaths(inputs, `Rank input shard ${basename(input)}`); + const outputs = loadRankRows(output, true, "Rank output shard"); + requireUniquePaths(outputs, `Rank output shard ${basename(output)}`); + const expected = new Map(inputs.map((row) => [row.path, row.area])); + const actual = new Set(outputs.map((row) => row.path)); + const missing = [...expected.keys()] + .filter((path) => !actual.has(path)) + .sort(compare); + const unknown = [...actual] + .filter((path) => !expected.has(path)) + .sort(compare); + if (missing.length || unknown.length) + throw new Error( + `${output}: paths do not match its input shard; missing=${pythonRepr(missing)}; unknown=${pythonRepr(unknown)}`, + ); + for (const row of outputs) + if (row.area !== expected.get(row.path)) + throw new Error( + `${output}: area does not match rank input for ${row.path}`, + ); + return [inputs, outputs]; +} + +function makeShards( + inputArgument: string, + directoryArgument: string, + maximum: bigint, + posixHome: string | undefined, +): void { + if (maximum < 1n) throw new Error("--max-rows must be at least 1"); + const input = worklistPath(inputArgument, posixHome); + const rows = loadRankRows(input, false); + requireUniquePaths(rows, "Rank input"); + const directory = worklistPath(directoryArgument, posixHome); + mkdir(directory); + if ( + shardNames(directory, "input").length || + shardNames(directory, "output").length + ) + throw new Error( + `Rank shard directory already contains shard files: ${directory}`, + ); + let count = 0; + const size = Number(maximum); + for (let start = 0; start < rows.length; start += size) { + const name = `rank-shard-${String(++count).padStart(4, "0")}.input.jsonl`; + writeRankRows(childPath(directory, name), rows.slice(start, start + size)); + } + print(`Wrote ${count} rank shards to ${directory}`); +} + +function mergeShards( + inputArgument: string, + directoryArgument: string, + outputArgument: string, + posixHome: string | undefined, +): void { + const input = worklistPath(inputArgument, posixHome); + const authoritative = loadRankRows(input, false); + requireUniquePaths(authoritative, "Rank input"); + const directory = worklistPath(directoryArgument, posixHome); + const shards = discoverInputShards(directory); + const expected = new Set( + shards.map((path) => + basename(path).replaceAll(".input.jsonl", ".output.jsonl"), + ), + ); + const actual = new Set(shardNames(directory, "output")); + const missing = [...expected] + .filter((name) => !actual.has(name)) + .sort(compare); + const unexpected = [...actual] + .filter((name) => !expected.has(name)) + .sort(compare); + if (missing.length || unexpected.length) { + const details: string[] = []; + if (missing.length) + details.push(`missing output shards ${pythonRepr(missing)}`); + if (unexpected.length) + details.push(`unexpected output shards ${pythonRepr(unexpected)}`); + throw new Error(`Rank shard outputs are incomplete: ${details.join("; ")}`); + } + const shardedInputs: RankRow[] = []; + const outputByPath = new Map(); + for (const shard of shards) { + const outputShard = childPath( + directory, + basename(shard).replaceAll(".input.jsonl", ".output.jsonl"), + ); + const [inputs, outputs] = validateShard(shard, outputShard); + for (const row of inputs) shardedInputs.push(row); + for (const row of outputs) { + if (outputByPath.has(row.path)) + throw new Error(`Rank outputs contain duplicate path: ${row.path}`); + outputByPath.set(row.path, row); + } + } + if ( + shardedInputs.length !== authoritative.length || + shardedInputs.some((row, index) => { + const expectedRow = authoritative[index]!; + return ( + row.path !== expectedRow.path || + row.area !== expectedRow.area || + row.preview !== expectedRow.preview + ); + }) + ) + throw new Error( + "Rank input shards do not exactly partition the authoritative rank input", + ); + const merged = authoritative.map((row) => outputByPath.get(row.path)!); + const output = worklistPath(outputArgument, posixHome); + writeRankRows(output, merged); + print(`Merged ${merged.length} ranking rows into ${output}`); +} + +export function rankShardsCommand( + command: Command, + args: string[], + posixHome = process.env.HOME, +): number { + const required = + command === "make-rank-shards" + ? ["rank-input", "out-dir"] + : command === "validate-rank-shard" + ? ["input", "output"] + : ["rank-input", "shard-dir", "out"]; + const integers = command === "make-rank-shards" ? ["max-rows"] : []; + const usage = `usage: launch_codex_security_mcp[.cmd] --helper ${command} [-h] ${required.map((name) => `--${name} PATH`).join(" ")}${integers.length ? " [--max-rows INT]" : ""}`; + try { + const values = argumentsFor(args, required, integers); + if (values.help) { + const description = + command === "make-rank-shards" + ? "Partition rank_input.jsonl into deterministic worker input shards." + : command === "validate-rank-shard" + ? "Validate one worker output against its rank input shard." + : "Validate worker shard outputs and create rank_output.jsonl."; + print( + `${usage}\n\n${description}\n\noptions:\n -h, --help show this help message and exit\n${required.map((name) => ` --${name} PATH`).join("\n")}${integers.length ? "\n --max-rows INT Maximum rows per shard. Defaults to 150." : ""}`, + ); + return 0; + } + if (command === "make-rank-shards") { + const maximum = (values["max-rows"] ?? 150n) as bigint; + makeShards( + values["rank-input"] as string, + values["out-dir"] as string, + maximum, + posixHome, + ); + } else if (command === "validate-rank-shard") { + const input = worklistPath(values.input as string, posixHome); + const output = worklistPath(values.output as string, posixHome); + const [, rows] = validateShard(input, output); + print(`Validated ${rows.length} ranking rows in ${output}`); + } else { + mergeShards( + values["rank-input"] as string, + values["shard-dir"] as string, + values.out as string, + posixHome, + ); + } + return 0; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + print( + error instanceof ArgumentError + ? `${usage}\n${command}: error: ${message}` + : message, + true, + ); + return error instanceof ArgumentError ? 2 : 1; + } +} diff --git a/plugins/codex-security/mcp-app/src/helpers/rank-worklists.ts b/plugins/codex-security/mcp-app/src/helpers/rank-worklists.ts new file mode 100644 index 000000000..9f012f6de --- /dev/null +++ b/plugins/codex-security/mcp-app/src/helpers/rank-worklists.ts @@ -0,0 +1,232 @@ +import { decodeUtf8 } from "./utf8"; +import { dirname } from "node:path"; +import { exists, mkdir, readFile, writeFile } from "./helper-files"; +import { encodePosixPath } from "./posix-path"; +import { JsonSyntaxError, object, parseJson, pythonRepr } from "./python-json"; +import { expandHome, parsedPath } from "./resolve-security-md"; + +export interface RankRow { + path: string; + area: string; + preview?: string; + reason?: string; + score?: bigint; + include?: boolean; +} +const trim = (value: string) => + value.replace( + /^[\p{White_Space}\u001c-\u001f]+|[\p{White_Space}\u001c-\u001f]+$/gu, + "", + ); +export function compare(left: string, right: string): number { + const a = Array.from(left, (character) => character.codePointAt(0)!); + const b = Array.from(right, (character) => character.codePointAt(0)!); + for (let index = 0; index < Math.min(a.length, b.length); index++) { + if (a[index] !== b[index]) return a[index]! - b[index]!; + } + return a.length - b.length; +} + +export function loadRankRows( + path: string, + selection: boolean, + label = selection ? "Rank output" : "Rank input", +): RankRow[] { + if (!exists(path)) throw new Error(`${label} missing: ${path}`); + const contents = decodeUtf8(readFile(path)); + const lines = contents === "" ? [] : contents.split(/\r\n|[\r\n]/u); + if (lines.at(-1) === "") lines.pop(); + const fields = selection + ? ["path", "area", "score", "include", "reason"] + : ["path", "area", "preview"]; + const rows = lines.map((line, index) => { + const fail = (message: string): never => { + throw new Error(`${path}:${index + 1}: ${message}`); + }; + if (trim(line) === "") fail("blank JSONL rows are not allowed"); + let row: unknown; + try { + row = parseJson(line); + } catch (error) { + if (error instanceof JsonSyntaxError) + fail( + `invalid JSON: ${error.message.replace(/: line \d+ column \d+ \(char \d+\)$/u, "")}`, + ); + throw error; + } + if (!object(row)) return fail("expected a JSON object"); + const missing = fields + .filter((field) => !Object.hasOwn(row, field)) + .sort(compare); + const unexpected = Object.keys(row) + .filter((field) => !fields.includes(field)) + .sort(compare); + const details: string[] = []; + if (missing.length) details.push(`missing fields ${pythonRepr(missing)}`); + if (unexpected.length) + details.push(`unexpected fields ${pythonRepr(unexpected)}`); + if (details.length) fail(details.join("; ")); + for (const field of selection + ? ["path", "area"] + : ["path", "area", "preview"]) { + if ( + typeof row[field] !== "string" || + (field === "path" && trim(row[field]) === "") + ) + fail( + `${field} must be ${field === "path" ? "a non-empty string" : "a string"}`, + ); + } + if (selection) { + if (typeof row.score !== "bigint") + fail("score must be an integer from 1 through 10"); + if ((row.score as bigint) < 1n || (row.score as bigint) > 10n) + fail("score must be from 1 through 10"); + if (typeof row.include !== "boolean") fail("include must be a boolean"); + if (typeof row.reason !== "string" || trim(row.reason) === "") + fail("reason must be a non-empty string"); + } + return row as unknown as RankRow; + }); + return rows; +} + +export function requireUniquePaths(rows: RankRow[], label: string): void { + const seen = new Set(), + duplicates = new Set(); + for (const row of rows) { + if (seen.has(row.path)) duplicates.add(row.path); + seen.add(row.path); + } + if (duplicates.size) + throw new Error( + `${label} contains duplicate paths: ${pythonRepr([...duplicates].sort(compare))}`, + ); +} + +export function writeRankRows(output: string, rows: RankRow[]): void { + mkdir(dirname(output)); + function* contents(): Iterable { + for (const row of rows) { + const json = JSON.stringify(row, (_key, value: unknown) => + typeof value === "bigint" ? Number(value) : value, + ).replace( + /[\u007f-\uffff]/g, + (character) => + `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, + ); + yield Buffer.from(json + (process.platform === "win32" ? "\r\n" : "\n")); + } + } + writeFile(output, contents()); +} + +export class ArgumentError extends Error {} +function integer(value: string, option: string): bigint { + const text = value.replace(/^\p{White_Space}+|\p{White_Space}+$/gu, ""); + if (!/^[+-]?\p{Decimal_Number}+(?:_\p{Decimal_Number}+)*$/u.test(text)) + throw new ArgumentError( + `argument --${option}: invalid int value: ${pythonRepr(value)}`, + ); + return BigInt( + Array.from(text.replaceAll("_", ""), (character) => { + if (!/\p{Decimal_Number}/u.test(character)) return character; + const point = character.codePointAt(0)!; + let start = point; + while (/\p{Decimal_Number}/u.test(String.fromCodePoint(start - 1))) + start--; + return String((point - start) % 10); + }).join(""), + ); +} + +export function argumentsFor( + args: string[], + required: readonly string[], + integerOptions: readonly string[] = [], +): Record { + const names = [...required, "help", ...integerOptions]; + const values: Record = {}; + const extra: string[] = []; + const looksOptional = (arg: string) => + arg.startsWith("-") && + arg !== "-" && + !arg.includes(" ") && + !/^-(?:\p{Decimal_Number}+|\p{Decimal_Number}*\.\p{Decimal_Number}+)\n?$/u.test( + arg, + ); + for (let index = 0; index < args.length; index++) { + const arg = args[index]!; + if (arg === "--") { + extra.push(...args.slice(index)); + break; + } + const equals = arg.indexOf("="); + const option = equals === -1 ? arg : arg.slice(0, equals); + const matches = option.startsWith("--") + ? names.filter((name) => `--${name}`.startsWith(option)) + : []; + const name = + names.find((name) => option === `--${name}`) ?? + (arg.startsWith("-h") + ? "help" + : matches.length === 1 + ? matches[0] + : undefined); + if (matches.length > 1 && !name) + throw new ArgumentError( + `ambiguous option: ${arg} could match ${matches.map((name) => `--${name}`).join(", ")}`, + ); + if (!name) { + extra.push(arg); + continue; + } + if (name === "help") { + if (equals !== -1) + throw new ArgumentError( + `argument -h/--help: ignored explicit argument ${pythonRepr(arg.slice(equals + 1))}`, + ); + return { help: true }; + } + let value: string; + if (equals !== -1) value = arg.slice(equals + 1); + else { + if (index + 1 === args.length || looksOptional(args[index + 1]!)) + throw new ArgumentError(`argument --${name}: expected one argument`); + value = args[++index]!; + } + values[name] = integerOptions.includes(name) ? integer(value, name) : value; + } + const missing = required.filter((name) => values[name] === undefined); + if (missing.length) + throw new ArgumentError( + `the following arguments are required: ${missing.map((name) => `--${name}`).join(", ")}`, + ); + if (extra.length) + throw new ArgumentError(`unrecognized arguments: ${extra.join(" ")}`); + return values; +} + +export function print(message: string, stderr = false): void { + let text = message + "\n"; + if (stderr) + text = text.replace( + /[\ud800-\udfff]/gu, + (character) => + `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, + ); + (stderr ? process.stderr : process.stdout).write( + process.platform === "win32" + ? text.replaceAll("\n", "\r\n") + : stderr + ? text + : encodePosixPath(text), + ); +} + +export function worklistPath( + value: string, + posixHome: string | undefined, +): string { + return parsedPath(expandHome(parsedPath(value), posixHome)); +} diff --git a/plugins/codex-security/native/README.md b/plugins/codex-security/native/README.md index 8eee15c3b..129de7f9c 100644 --- a/plugins/codex-security/native/README.md +++ b/plugins/codex-security/native/README.md @@ -41,7 +41,7 @@ Windows uses `windows-binding.mts` and the same Rust crate. `WindowsHandle` owns The binding exposes synchronous file and directory creation, attributes and reparse tags, identity and final/opened names, read/write/seek/size/EOF/flush, exact-handle rename and deletion, and exclusive whole-file locking. Rust's `File` supplies ordinary I/O, cursor-preserving truncation, `sync_all` for flush, and locks. Calls return numeric Windows errors, including 6 for closed handles and 33 for nonblocking lock contention. Buffer ranges, path encoding, and 64-bit seek arguments are checked before use. Overlapped handles are unsupported because pending operations could retain native buffers beyond the call. Path authorization, ancestor traversal, and reparse-point policy remain the caller's responsibility. -Five additional operations preserve Windows strings at the Node boundary. `windowsArguments` returns the complete OS argument vector, including the executable and Node options, using Rust's CRT-compatible parser. `windowsEnvironment` reads one wide environment name and distinguishes an absent value (`null`) from an empty buffer. `windowsAbsolutePath` resolves against the native current directory and drive directories without requiring the destination to exist. `windowsDirectoryEntries` uses `std::fs::read_dir` and cached `DirEntry::file_type()` values without opening each child; names remain UTF-16LE, and construction or iteration failures return their numeric Windows error and an empty array. Directory symlinks and junctions have both directory and symbolic-link flags. The typed adapter exposes this enumerator through `entriesWithTypes`, which `resolve-security-md --list` uses on Windows. `windowsReadLink` returns a UTF-16LE link target or its numeric Windows error; candidate normalization uses it to resolve missing paths without losing raw filenames. Assessment validation and deep-review worklists share the typed wide-path reader and writer. +Five additional operations preserve Windows strings at the Node boundary. `windowsArguments` returns the complete OS argument vector, including the executable and Node options, using Rust's CRT-compatible parser. `windowsEnvironment` reads one wide environment name and distinguishes an absent value (`null`) from an empty buffer. `windowsAbsolutePath` resolves against the native current directory and drive directories without requiring the destination to exist. `windowsDirectoryEntries` uses `std::fs::read_dir` and cached `DirEntry::file_type()` values without opening each child; names remain UTF-16LE, and construction or iteration failures return their numeric Windows error and an empty array. Directory symlinks and junctions have both directory and symbolic-link flags. The typed adapter exposes this enumerator through `entriesWithTypes`, which `resolve-security-md --list` uses on Windows. `windowsReadLink` returns a UTF-16LE link target or its numeric Windows error; candidate normalization uses it to resolve missing paths without losing raw filenames. Assessment validation, deep-review worklists, and rank shards share these typed wide-path operations. `windows-files.mts` leaves ordinary absolute-path resolution and canonicalization to `GetFullPathNameW` and `GetFinalPathNameByHandleW`, trimming trailing separators below the root. Its small verbatim-path normalizer preserves drive and UNC share roots when resolving dot segments, including literal trailing dots and spaces. Non-strict `realpath` can retain unresolved components; callers must check containment independently. It also supports missing output paths. `stat(path, false)` retains exact symbolic-link and reparse-point metadata so callers can reject junction traversal independently of the enumerator's link label. The SDK's public runtime floor remains Node 22.13.0. Node 20.0.0 is an additional native-foundation compatibility proof; it does not change the SDK engine requirement. diff --git a/plugins/codex-security/native/examples/windows-wide-launcher.rs b/plugins/codex-security/native/examples/windows-wide-launcher.rs index 8fd307e1d..139c10f0d 100644 --- a/plugins/codex-security/native/examples/windows-wide-launcher.rs +++ b/plugins/codex-security/native/examples/windows-wide-launcher.rs @@ -425,7 +425,109 @@ fn main() -> std::io::Result<()> { "Deep-review helper changed a replacement path", )); } - println!("{{\"policyHelperRawPaths\":true,\"candidateHelperRawPaths\":true,\"assessmentHelperRawPaths\":true,\"deepReviewHelperRawPaths\":true,\"directoryIdentity\":true}}"); + let shards_name = raw("shards-", 0xdc80); + let shards = repo.join(&shards_name); + let replacement_shards = repo.join(raw("shards-", 0xfffd)); + fs::create_dir(&replacement_shards)?; + let shard_sentinel = replacement_shards.join("rank-shard-0001.input.jsonl"); + fs::write(&shard_sentinel, "replacement shard sentinel")?; + let merge_parent = raw("merged-", 0xd800); + let merged = repo.join(&merge_parent).join(&output_name); + let merge_sentinel = repo.join(raw("merged-", 0xfffd)).join(&replacement_output); + fs::create_dir_all(merge_sentinel.parent().unwrap())?; + fs::write(&merge_sentinel, "replacement merge sentinel")?; + let input_rows = [ + "{\"path\":\"first.py\",\"area\":\"src\",\"preview\":\"first\"}\r\n", + "{\"path\":\"second.py\",\"area\":\"src\",\"preview\":\"second\"}\r\n", + ]; + let output_rows = [ + "{\"path\":\"first.py\",\"area\":\"src\",\"score\":5,\"include\":true,\"reason\":\"review\"}\r\n", + "{\"path\":\"second.py\",\"area\":\"src\",\"score\":6,\"include\":true,\"reason\":\"review\"}\r\n", + ]; + fs::write(repo.join(&input_name), input_rows.concat())?; + let shard_command = |command: &str, args: &[PathBuf]| { + Command::new(&node) + .arg(&script) + .args(["--helper", command]) + .args(args) + .current_dir(&repo) + .env("USERPROFILE", &repo) + .output() + }; + let made = shard_command( + "make-rank-shards", + &[ + "--rank-input".into(), + repo.join(&input_name), + "--out-dir".into(), + Path::new("~").join(&shards_name), + "--max-rows".into(), + "1".into(), + ], + )?; + if !made.status.success() || !made.stderr.is_empty() { + return Err(io::Error::other(format!( + "Wide shard creation failed: {}", + String::from_utf8_lossy(&made.stderr) + ))); + } + for (index, input_row) in input_rows.iter().enumerate() { + let stem = format!("rank-shard-{:04}", index + 1); + if fs::read(shards.join(format!("{stem}.input.jsonl")))? != input_row.as_bytes() { + return Err(io::Error::other("Wide input shard bytes changed")); + } + fs::write( + shards.join(format!("{stem}.output.jsonl")), + output_rows[index], + )?; + } + let validated = shard_command( + "validate-rank-shard", + &[ + "--input".into(), + Path::new(&shards_name).join("rank-shard-0001.input.jsonl"), + "--output".into(), + Path::new(&shards_name).join("rank-shard-0001.output.jsonl"), + ], + )?; + let merge_args = [ + "--rank-input".into(), + Path::new(".").join(&input_name), + "--shard-dir".into(), + shards.clone(), + "--out".into(), + Path::new("~").join(&merge_parent).join(&output_name), + ]; + let merge_result = shard_command("merge-rank-outputs", &merge_args)?; + if !validated.status.success() + || !validated.stderr.is_empty() + || !merge_result.status.success() + || !merge_result.stderr.is_empty() + || fs::read(&merged)? != output_rows.concat().as_bytes() + { + return Err(io::Error::other(format!( + "Wide shard validation/merge failed: {}{}", + String::from_utf8_lossy(&validated.stderr), + String::from_utf8_lossy(&merge_result.stderr) + ))); + } + let mut malformed_name = raw("rank-shard-", 0xdfff); + malformed_name.push(".input.jsonl"); + fs::write(shards.join(&malformed_name), "")?; + let malformed = shard_command("merge-rank-outputs", &merge_args)?; + if malformed.status.code() != Some(1) + || !String::from_utf8_lossy(&malformed.stderr) + .contains("Rank input shard has invalid name: rank-shard-\\udfff.input.jsonl") + || fs::read(&merged)? != output_rows.concat().as_bytes() + || fs::read(&shard_sentinel)? != b"replacement shard sentinel" + || fs::read(&merge_sentinel)? != b"replacement merge sentinel" + || fs::read(repo.join(raw("input-", 0xfffd)))? != b"invalid replacement input" + { + return Err(io::Error::other( + "Wide shard discovery or replacement paths changed", + )); + } + println!("{{\"policyHelperRawPaths\":true,\"candidateHelperRawPaths\":true,\"assessmentHelperRawPaths\":true,\"deepReviewHelperRawPaths\":true,\"rankShardHelperRawPaths\":true,\"directoryIdentity\":true}}"); Ok(()) } diff --git a/plugins/codex-security/scripts/generate_rank_input.py b/plugins/codex-security/scripts/generate_rank_input.py index a9cfa2add..21cbb1296 100644 --- a/plugins/codex-security/scripts/generate_rank_input.py +++ b/plugins/codex-security/scripts/generate_rank_input.py @@ -8,15 +8,11 @@ - `make-diff-rank-input` creates the deterministic diff-scoped JSONL candidate worklist from Git changed paths. It supports committed revision diffs and local working-tree patches. -- `make-rank-shards` partitions the ranking input into deterministic shards. - `make-rank-pool-plan` assigns those shards to a deterministic bounded worker pool. - `validate-rank-worker` validates one worker slot and emits a content-bound completion receipt. -- `validate-rank-shard` validates one completed worker output before the - coordinator accepts it. - `validate-rank-pool` validates the pool plan and every assigned shard output. -- `merge-rank-outputs` validates and combines worker-local shard outputs. """ from __future__ import annotations @@ -199,19 +195,6 @@ def parse_args() -> argparse.Namespace: help=f"Maximum UTF-8 bytes in each preview. Defaults to {DEFAULT_PREVIEW_BYTES}.", ) - shards = subparsers.add_parser( - "make-rank-shards", - help="Partition rank_input.jsonl into deterministic worker input shards.", - ) - shards.add_argument("--rank-input", required=True, help="Deterministic rank input JSONL.") - shards.add_argument("--out-dir", required=True, help="Directory for worker input shards.") - shards.add_argument( - "--max-rows", - type=int, - default=150, - help="Maximum rows per shard. Defaults to 150.", - ) - pool_plan = subparsers.add_parser( "make-rank-pool-plan", help="Assign rank shards to a deterministic bounded worker pool.", @@ -225,13 +208,6 @@ def parse_args() -> argparse.Namespace: ) pool_plan.add_argument("--out", required=True, help="Output rank_worker_assignments.json path.") - validate_shard = subparsers.add_parser( - "validate-rank-shard", - help="Validate one worker output against its rank input shard.", - ) - validate_shard.add_argument("--input", required=True, help="Worker rank input shard.") - validate_shard.add_argument("--output", required=True, help="Worker rank output shard.") - validate_worker = subparsers.add_parser( "validate-rank-worker", help="Validate one assigned ranking-worker slot and emit its completion receipt.", @@ -252,14 +228,6 @@ def parse_args() -> argparse.Namespace: validate_pool.add_argument("--plan", required=True, help="Rank pool plan JSON path.") validate_pool.add_argument("--shard-dir", required=True, help="Directory of rank shards.") - merge = subparsers.add_parser( - "merge-rank-outputs", - help="Validate worker shard outputs and create rank_output.jsonl.", - ) - merge.add_argument("--rank-input", required=True, help="Authoritative rank input JSONL.") - merge.add_argument("--shard-dir", required=True, help="Directory of input and output shards.") - merge.add_argument("--out", required=True, help="Output rank_output.jsonl path.") - return parser.parse_args() @@ -715,29 +683,6 @@ def make_diff_rank_input(args: argparse.Namespace) -> None: print(f"Wrote {len(rows)} rows to {output}") -def make_rank_shards(args: argparse.Namespace) -> None: - if args.max_rows < 1: - raise SystemExit("--max-rows must be at least 1") - - rank_input = Path(args.rank_input).expanduser() - rows = load_jsonl(rank_input, "Rank input", validate_rank_input_row) - require_unique_paths(rows, "Rank input") - - output_dir = Path(args.out_dir).expanduser() - output_dir.mkdir(parents=True, exist_ok=True) - existing = sorted((*output_dir.glob(SHARD_INPUT_GLOB), *output_dir.glob(SHARD_OUTPUT_GLOB))) - if existing: - raise SystemExit(f"Rank shard directory already contains shard files: {output_dir}") - - shard_count = 0 - for start in range(0, len(rows), args.max_rows): - shard_count += 1 - shard_path = output_dir / f"rank-shard-{shard_count:04d}.input.jsonl" - write_jsonl(shard_path, rows[start : start + args.max_rows]) - - print(f"Wrote {shard_count} rank shards to {output_dir}") - - def discover_input_shards(shard_dir: Path) -> list[Path]: if not shard_dir.is_dir(): raise SystemExit(f"Rank shard directory missing: {shard_dir}") @@ -1058,58 +1003,6 @@ def validate_rank_shard( return shard_inputs, shard_outputs -def validate_rank_shard_command(args: argparse.Namespace) -> None: - input_shard = Path(args.input).expanduser() - output_shard = Path(args.output).expanduser() - _, output_rows = validate_rank_shard(input_shard, output_shard) - print(f"Validated {len(output_rows)} ranking rows in {output_shard}") - - -def merge_rank_outputs(args: argparse.Namespace) -> None: - rank_input = Path(args.rank_input).expanduser() - authoritative_rows = load_jsonl(rank_input, "Rank input", validate_rank_input_row) - require_unique_paths(authoritative_rows, "Rank input") - - shard_dir = Path(args.shard_dir).expanduser() - input_shards = discover_input_shards(shard_dir) - output_shards = sorted(shard_dir.glob(SHARD_OUTPUT_GLOB)) - expected_output_names = { - path.name.replace(".input.jsonl", ".output.jsonl") for path in input_shards - } - actual_output_names = {path.name for path in output_shards} - if expected_output_names != actual_output_names: - missing = sorted(expected_output_names - actual_output_names) - unexpected = sorted(actual_output_names - expected_output_names) - details: list[str] = [] - if missing: - details.append(f"missing output shards {missing}") - if unexpected: - details.append(f"unexpected output shards {unexpected}") - raise SystemExit(f"Rank shard outputs are incomplete: {'; '.join(details)}") - - sharded_inputs: list[JsonRow] = [] - output_by_path: dict[str, JsonRow] = {} - for input_shard in input_shards: - output_shard = input_shard.with_name( - input_shard.name.replace(".input.jsonl", ".output.jsonl") - ) - shard_inputs, shard_outputs = validate_rank_shard(input_shard, output_shard) - sharded_inputs.extend(shard_inputs) - for row in shard_outputs: - row_path = str(row["path"]) - if row_path in output_by_path: - raise SystemExit(f"Rank outputs contain duplicate path: {row_path}") - output_by_path[row_path] = row - - if sharded_inputs != authoritative_rows: - raise SystemExit("Rank input shards do not exactly partition the authoritative rank input") - - merged = [output_by_path[str(row["path"])] for row in authoritative_rows] - output = Path(args.out).expanduser() - write_jsonl(output, merged) - print(f"Merged {len(merged)} ranking rows into {output}") - - def main() -> None: args = parse_args() if args.command == "make-repo-rank-input": @@ -1120,18 +1013,12 @@ def main() -> None: bind_repo_scopes(args) elif args.command == "make-diff-rank-input": make_diff_rank_input(args) - elif args.command == "make-rank-shards": - make_rank_shards(args) elif args.command == "make-rank-pool-plan": make_rank_pool_plan(args) - elif args.command == "validate-rank-shard": - validate_rank_shard_command(args) elif args.command == "validate-rank-worker": validate_rank_worker_command(args) elif args.command == "validate-rank-pool": validate_rank_pool_command(args) - elif args.command == "merge-rank-outputs": - merge_rank_outputs(args) else: raise SystemExit(f"Unknown command: {args.command}") diff --git a/plugins/codex-security/tests/test_generate_rank_input.py b/plugins/codex-security/tests/test_generate_rank_input.py index cb7dce59e..2d05495cb 100644 --- a/plugins/codex-security/tests/test_generate_rank_input.py +++ b/plugins/codex-security/tests/test_generate_rank_input.py @@ -89,17 +89,12 @@ def make_shards_and_pool_plan( tmp_path: Path, *, shard_count: int = 5, usable_worker_slots: int = 2 ) -> tuple[Path, Path, Path]: rank_input = tmp_path / "rank_input.jsonl" - write_jsonl(rank_input, make_rank_rows(shard_count)) + rows = make_rank_rows(shard_count) + write_jsonl(rank_input, rows) shard_dir = tmp_path / "rank_shards" - run_cli( - "make-rank-shards", - "--rank-input", - str(rank_input), - "--max-rows", - "1", - "--out-dir", - str(shard_dir), - ) + shard_dir.mkdir() + for index, row in enumerate(rows, start=1): + write_jsonl(shard_dir / f"rank-shard-{index:04d}.input.jsonl", [row]) plan = tmp_path / "rank_worker_assignments.json" run_cli( "make-rank-pool-plan", @@ -860,41 +855,6 @@ def test_make_rank_input_decodes_bom_marked_utf16_source(tmp_path: Path, mode: s assert {row["path"]: row["preview"] for row in read_jsonl(output)} == expected -def test_make_rank_shards_is_deterministic_and_bounded(tmp_path: Path) -> None: - rank_input = tmp_path / "rank_input.jsonl" - rows = make_rank_rows(312) - write_jsonl(rank_input, rows) - shard_dir = tmp_path / "shards" - - run_cli( - "make-rank-shards", - "--rank-input", - str(rank_input), - "--out-dir", - str(shard_dir), - ) - - shards = sorted(shard_dir.glob("*.input.jsonl")) - assert [path.name for path in shards] == [ - "rank-shard-0001.input.jsonl", - "rank-shard-0002.input.jsonl", - "rank-shard-0003.input.jsonl", - ] - assert [len(read_jsonl(path)) for path in shards] == [150, 150, 12] - assert [row for path in shards for row in read_jsonl(path)] == rows - - result = run_cli( - "make-rank-shards", - "--rank-input", - str(rank_input), - "--out-dir", - str(shard_dir), - check=False, - ) - assert result.returncode != 0 - assert "already contains shard files" in result.stderr - - def test_make_rank_pool_plan_is_deterministic_round_robin_and_exact_once( tmp_path: Path, ) -> None: @@ -974,19 +934,9 @@ def test_make_rank_pool_plan_caps_workers_at_six(tmp_path: Path) -> None: ] -def test_empty_rank_input_closes_with_zero_shards_and_workers(tmp_path: Path) -> None: - rank_input = tmp_path / "rank_input.jsonl" - write_jsonl(rank_input, []) +def test_empty_rank_pool_closes_with_zero_shards_and_workers(tmp_path: Path) -> None: shard_dir = tmp_path / "rank_shards" - - shards_result = run_cli( - "make-rank-shards", - "--rank-input", - str(rank_input), - "--out-dir", - str(shard_dir), - ) - assert shards_result.stdout == f"Wrote 0 rank shards to {shard_dir}\n" + shard_dir.mkdir() plan = tmp_path / "rank_worker_assignments.json" plan_result = run_cli( @@ -1016,33 +966,11 @@ def test_empty_rank_input_closes_with_zero_shards_and_workers(tmp_path: Path) -> ) assert pool_result.stdout == "Validated 0 ranking workers, 0 shards, and 0 ranking rows\n" - rank_output = tmp_path / "rank_output.jsonl" - merge_result = run_cli( - "merge-rank-outputs", - "--rank-input", - str(rank_input), - "--shard-dir", - str(shard_dir), - "--out", - str(rank_output), - ) - assert merge_result.stdout == f"Merged 0 ranking rows into {rank_output}\n" - assert rank_output.read_bytes() == b"" - def test_make_rank_pool_plan_requires_sibling_rank_shards_directory(tmp_path: Path) -> None: - rank_input = tmp_path / "rank_input.jsonl" - write_jsonl(rank_input, make_rank_rows(2)) shard_dir = tmp_path / "other_shards" - run_cli( - "make-rank-shards", - "--rank-input", - str(rank_input), - "--max-rows", - "1", - "--out-dir", - str(shard_dir), - ) + for index, row in enumerate(make_rank_rows(2), start=1): + write_jsonl(shard_dir / f"rank-shard-{index:04d}.input.jsonl", [row]) plan = tmp_path / "rank_worker_assignments.json" result = run_cli( @@ -1301,9 +1229,7 @@ def test_validate_rank_pool_accepts_complete_multi_shard_workers(tmp_path: Path) def test_rank_pool_accepts_parent_completion_for_an_unstarted_worker_slot( tmp_path: Path, ) -> None: - rank_input, shard_dir, plan = make_shards_and_pool_plan( - tmp_path, shard_count=5, usable_worker_slots=2 - ) + _, shard_dir, plan = make_shards_and_pool_plan(tmp_path, shard_count=5, usable_worker_slots=2) write_worker_shard_outputs(shard_dir, plan, slot=1) write_worker_shard_outputs(shard_dir, plan, slot=2) @@ -1326,21 +1252,7 @@ def test_rank_pool_accepts_parent_completion_for_an_unstarted_worker_slot( "--shard-dir", str(shard_dir), ) - rank_output = tmp_path / "rank_output.jsonl" - run_cli( - "merge-rank-outputs", - "--rank-input", - str(rank_input), - "--shard-dir", - str(shard_dir), - "--out", - str(rank_output), - ) - assert "Validated 2 ranking workers, 5 shards, and 5 ranking rows" in pool.stdout - assert [row["path"] for row in read_jsonl(rank_output)] == [ - row["path"] for row in read_jsonl(rank_input) - ] def test_validate_rank_pool_rejects_missing_and_unexpected_outputs(tmp_path: Path) -> None: @@ -1417,170 +1329,3 @@ def test_validate_rank_pool_rejects_duplicate_output_rows(tmp_path: Path) -> Non assert result.returncode != 0 assert "contains duplicate paths" in result.stderr - - -def test_validate_rank_shard_validates_one_pool_output(tmp_path: Path) -> None: - rank_input = tmp_path / "rank_input.jsonl" - write_jsonl(rank_input, make_rank_rows(7)) - shard_dir = tmp_path / "shards" - run_cli( - "make-rank-shards", - "--rank-input", - str(rank_input), - "--max-rows", - "5", - "--out-dir", - str(shard_dir), - ) - first_input = shard_dir / "rank-shard-0001.input.jsonl" - first_output = shard_dir / "rank-shard-0001.output.jsonl" - write_jsonl(first_output, [rank_result(row) for row in read_jsonl(first_input)]) - - result = run_cli( - "validate-rank-shard", - "--input", - str(first_input), - "--output", - str(first_output), - ) - - assert "Validated 5 ranking rows" in result.stdout - assert not (shard_dir / "rank-shard-0002.output.jsonl").exists() - - -def test_merge_rank_outputs_validates_and_restores_authoritative_order(tmp_path: Path) -> None: - rank_input = tmp_path / "rank_input.jsonl" - rows = make_rank_rows(7) - write_jsonl(rank_input, rows) - shard_dir = tmp_path / "shards" - run_cli( - "make-rank-shards", - "--rank-input", - str(rank_input), - "--max-rows", - "5", - "--out-dir", - str(shard_dir), - ) - for input_shard in sorted(shard_dir.glob("*.input.jsonl")): - output_shard = input_shard.with_name(input_shard.name.replace(".input.", ".output.")) - shard_rows = read_jsonl(input_shard) - write_jsonl(output_shard, [rank_result(row) for row in reversed(shard_rows)]) - - output = tmp_path / "rank_output.jsonl" - run_cli( - "merge-rank-outputs", - "--rank-input", - str(rank_input), - "--shard-dir", - str(shard_dir), - "--out", - str(output), - ) - - assert [row["path"] for row in read_jsonl(output)] == [row["path"] for row in rows] - - -@pytest.mark.parametrize( - ("output_text", "expected_error"), - [ - ("{not json}\n", "invalid JSON"), - ( - '{"path":"a.py","area":"core","score":10,"include":true}\n', - "missing fields ['reason']", - ), - ( - '{"path":"a.py","area":"core","score":true,"include":true,"reason":"x"}\n', - "score must be an integer", - ), - ( - '{"path":"a.py","area":"core","score":11,"include":true,"reason":"x"}\n', - "score must be from 1 through 10", - ), - ( - '{"path":"a.py","area":"core","score":10,"include":"true","reason":"x"}\n', - "include must be a boolean", - ), - ( - '{"path":"a.py","area":"core","score":10,"include":true,"reason":""}\n', - "reason must be a non-empty string", - ), - ( - '{"path":"b.py","area":"core","score":10,"include":true,"reason":"x"}\n', - "paths do not match its input shard", - ), - ( - "", - "paths do not match its input shard", - ), - ], -) -def test_merge_rank_outputs_rejects_invalid_worker_results( - tmp_path: Path, output_text: str, expected_error: str -) -> None: - rank_input = tmp_path / "rank_input.jsonl" - write_jsonl(rank_input, [{"path": "a.py", "area": "core", "preview": "a"}]) - shard_dir = tmp_path / "shards" - run_cli( - "make-rank-shards", - "--rank-input", - str(rank_input), - "--out-dir", - str(shard_dir), - ) - (shard_dir / "rank-shard-0001.output.jsonl").write_text(output_text, encoding="utf-8") - - result = run_cli( - "merge-rank-outputs", - "--rank-input", - str(rank_input), - "--shard-dir", - str(shard_dir), - "--out", - str(tmp_path / "rank_output.jsonl"), - check=False, - ) - - assert result.returncode != 0 - assert expected_error in result.stderr - - -def test_merge_rank_outputs_rejects_missing_and_duplicate_results(tmp_path: Path) -> None: - rank_input = tmp_path / "rank_input.jsonl" - rows = [{"path": "a.py", "area": "core", "preview": "a"}] - write_jsonl(rank_input, rows) - shard_dir = tmp_path / "shards" - run_cli( - "make-rank-shards", - "--rank-input", - str(rank_input), - "--out-dir", - str(shard_dir), - ) - - result = run_cli( - "merge-rank-outputs", - "--rank-input", - str(rank_input), - "--shard-dir", - str(shard_dir), - "--out", - str(tmp_path / "rank_output.jsonl"), - check=False, - ) - assert "missing output shards" in result.stderr - - output_shard = shard_dir / "rank-shard-0001.output.jsonl" - duplicate = rank_result(rows[0]) - write_jsonl(output_shard, [duplicate, duplicate]) - result = run_cli( - "merge-rank-outputs", - "--rank-input", - str(rank_input), - "--shard-dir", - str(shard_dir), - "--out", - str(tmp_path / "rank_output.jsonl"), - check=False, - ) - assert "duplicate paths" in result.stderr diff --git a/sdk/typescript/tests-ts/rank-shards.test.ts b/sdk/typescript/tests-ts/rank-shards.test.ts new file mode 100644 index 000000000..af977b13e --- /dev/null +++ b/sdk/typescript/tests-ts/rank-shards.test.ts @@ -0,0 +1,613 @@ +import { spawnSync } from "node:child_process"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + readdirSync, + realpathSync, + renameSync, + rmSync, + statSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join, sep } from "node:path"; +import { afterEach, describe, expect, test } from "bun:test"; +import { PLUGIN_ROOT } from "./plugin-root.js"; + +const node = Bun.which("node")!; +const helper = join(PLUGIN_ROOT, "mcp", "helpers.mjs"); +const roots: string[] = []; +const newline = process.platform === "win32" ? "\r\n" : "\n"; +type Row = Record; +const candidate = (path: string, area = "src") => ({ + path, + area, + preview: "source preview", +}); +const ranked = (row: Row) => ({ + path: row["path"], + area: row["area"], + score: 5, + include: true, + reason: "runtime surface", +}); +function fixture() { + const root = realpathSync(mkdtempSync(join(tmpdir(), "rank-shards-"))); + roots.push(root); + return { + root, + input: join(root, "input.jsonl"), + directory: join(root, "shards"), + output: join(root, "output.jsonl"), + }; +} +type Fixture = ReturnType; +function write(path: string, rows: Row[]) { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, rows.map((row) => JSON.stringify(row) + "\n").join("")); +} +function read(path: string): Row[] { + const text = readFileSync(path, "utf8").trim(); + return text === "" + ? [] + : text.split(/\r?\n/u).map((line) => JSON.parse(line) as Row); +} +function run(f: Fixture, command: string, args: string[], env = process.env) { + return spawnSync(node, [helper, command, ...args], { + cwd: f.root, + encoding: "utf8", + env, + input: "stdin is not a worklist", + }); +} +function make(f: Fixture, args: string[] = []) { + return run(f, "make-rank-shards", [ + "--rank-input", + f.input, + "--out-dir", + f.directory, + ...args, + ]); +} +function merge(f: Fixture, args: string[] = []) { + return run(f, "merge-rank-outputs", [ + "--rank-input", + f.input, + "--shard-dir", + f.directory, + "--out", + f.output, + ...args, + ]); +} +function shard(f: Fixture, index: number, output = false) { + return join( + f.directory, + `rank-shard-${String(index).padStart(4, "0")}.${output ? "output" : "input"}.jsonl`, + ); +} +function validate(f: Fixture, index = 1) { + return run(f, "validate-rank-shard", [ + "--input", + shard(f, index), + "--output", + shard(f, index, true), + ]); +} +function complete(f: Fixture) { + for (const name of readdirSync(f.directory).filter((name) => + name.endsWith(".input.jsonl"), + )) + write( + f.directory + sep + name.replace(".input.", ".output."), + read(f.directory + sep + name) + .reverse() + .map(ranked), + ); +} +afterEach(() => { + for (const root of roots.splice(0)) + rmSync(root, { recursive: true, force: true }); +}); + +describe("rank shard helpers", () => { + test.skipIf(process.platform !== "win32")( + "joins shard names beneath Windows roots without changing their path kind", + async () => { + const { childPath } = (await import( + new URL( + "../../../plugins/codex-security/mcp-app/src/helpers/rank-shards.ts", + import.meta.url, + ).href + )) as { childPath: (directory: string, name: string) => string }; + const name = "rank-shard-0001.input.jsonl"; + for (const [directory, prefix] of [ + ["/", "\\"], + ["\\", "\\"], + ["C:", "C:"], + ["C:\\", "C:\\"], + ["\\\\server\\share", "\\\\server\\share\\"], + ["\\\\?\\C:\\", "\\\\?\\C:\\"], + ["\\\\?\\UNC\\server\\share\\", "\\\\?\\UNC\\server\\share\\"], + ]) { + expect(childPath(directory!, name)).toBe(prefix! + name); + } + }, + ); + + test("partitions deterministically at the default 150 rows and refuses existing shards", () => { + const f = fixture(); + const rows = Array.from({ length: 312 }, (_, index) => + candidate(`src/file_${index}.py`), + ); + write(f.input, rows); + expect(make(f).stdout).toBe( + `Wrote 3 rank shards to ${f.directory}${newline}`, + ); + expect(readdirSync(f.directory).sort()).toEqual( + [1, 2, 3].map((index) => `rank-shard-000${index}.input.jsonl`), + ); + expect([1, 2, 3].map((index) => read(shard(f, index)).length)).toEqual([ + 150, 150, 12, + ]); + expect([1, 2, 3].flatMap((index) => read(shard(f, index)))).toEqual(rows); + const original = readFileSync(shard(f, 1)); + const second = make(f); + expect(second.status).toBe(1); + expect(second.stderr).toContain("already contains shard files"); + expect(readFileSync(shard(f, 1))).toEqual(original); + }); + + test.skipIf(process.platform === "win32")( + "preserves existing shards when Unicode entry types require metadata lookup", + () => { + const f = fixture(); + write(f.input, [candidate("new.py")]); + write(shard(f, 1), [candidate("preserved.py")]); + writeFileSync(join(f.directory, "é.txt"), "unrelated Unicode file"); + if (process.platform === "linux") + writeFileSync( + Buffer.concat([Buffer.from(f.directory + "/"), Buffer.from([255])]), + "unrelated undecodable file", + ); + const original = readFileSync(shard(f, 1)); + const preload = join(f.root, "unknown-types.cjs"); + writeFileSync( + preload, + `const fs = require("node:fs"); +const open = fs.opendirSync; +fs.opendirSync = (...args) => { + const handle = open(...args); + const read = handle.readSync.bind(handle); + handle.readSync = () => { + const entry = read(); + // Model DT_UNKNOWN lookup joining a Buffer directory with a string name. + if (entry) fs.lstatSync(require("node:path").join(args[0], entry.name)); + return entry; + }; + return handle; +}; +require("node:module").syncBuiltinESMExports(); +`, + ); + const invoke = () => + run( + f, + "make-rank-shards", + ["--rank-input", f.input, "--out-dir", f.directory], + { + ...process.env, + NODE_OPTIONS: `--require=${JSON.stringify(preload)}`, + }, + ); + const result = invoke(); + expect(result.status).toBe(1); + expect(result.stderr).toContain("already contains shard files"); + expect(readFileSync(shard(f, 1))).toEqual(original); + rmSync(shard(f, 1)); + expect(invoke().status).toBe(0); + expect(read(shard(f, 1))).toEqual([candidate("new.py")]); + }, + ); + + test("validates one completed shard independently and restores authoritative merge order", () => { + const f = fixture(); + const rows = Array.from({ length: 7 }, (_, index) => + candidate(`src/file_${index}.py`), + ); + write(f.input, rows); + expect(make(f, ["--max-rows", "5"]).status).toBe(0); + write(shard(f, 1, true), read(shard(f, 1)).reverse().map(ranked)); + const checked = validate(f); + expect(checked.status).toBe(0); + expect(checked.stdout).toBe( + `Validated 5 ranking rows in ${shard(f, 1, true)}${newline}`, + ); + expect(existsSync(shard(f, 2, true))).toBe(false); + complete(f); + const result = merge(f); + expect(result.status).toBe(0); + expect(result.stdout).toBe( + `Merged 7 ranking rows into ${f.output}${newline}`, + ); + expect(read(f.output)).toEqual(rows.map(ranked)); + }); + + test("creates zero shards and merges an empty input while ignoring unrelated files", () => { + const f = fixture(); + write(f.input, []); + mkdirSync(f.directory); + writeFileSync(join(f.directory, "notes.txt"), "retain notes"); + expect(make(f).stdout).toBe( + `Wrote 0 rank shards to ${f.directory}${newline}`, + ); + writeFileSync(f.output, "previous contents"); + expect(merge(f).stdout).toBe( + `Merged 0 ranking rows into ${f.output}${newline}`, + ); + expect(readFileSync(f.output)).toHaveLength(0); + expect(readFileSync(join(f.directory, "notes.txt"), "utf8")).toBe( + "retain notes", + ); + write(shard(f, 1), []); + write(shard(f, 1, true), []); + expect(validate(f).status).toBe(0); + }); + + test.each(["1", "+2_0", "２０", "٢٠", " 20 ", "9".repeat(400)])( + "accepts Python integer shard size %s", + (size) => { + const f = fixture(); + write(f.input, [candidate("a.py"), candidate("b.py")]); + expect(make(f, ["--max-rows", size]).status).toBe(0); + expect(readdirSync(f.directory)).toHaveLength(size === "1" ? 2 : 1); + }, + ); + + test.each(["0", "-1"])( + "rejects nonpositive shard size %s before input access", + (size) => { + const f = fixture(); + const result = make(f, ["--max-rows", size]); + expect(result.status).toBe(1); + expect(result.stderr).toBe(`--max-rows must be at least 1${newline}`); + expect(existsSync(f.directory)).toBe(false); + }, + ); + + test("keeps last repeated arguments, unique abbreviations, and argument failures", () => { + const f = fixture(); + write(f.input, [candidate("a.py"), candidate("b.py")]); + expect(make(f, ["--max-rows", "1", "--max-r=2"]).status).toBe(0); + expect(readdirSync(f.directory)).toHaveLength(1); + for (const args of [ + ["--max-rows", "1.0"], + ["--max-rows", "1__0"], + ["--max-rows", "\u001c20"], + ["--max-rows"], + ["--unknown"], + ["extra"], + ["--"], + ]) { + expect(make(f, args).status).toBe(2); + } + for (const command of [ + "make-rank-shards", + "validate-rank-shard", + "merge-rank-outputs", + ]) { + expect(run(f, command, []).status).toBe(2); + expect(run(f, command, ["--help"]).status).toBe(0); + } + expect(make(f, ["--help"]).stdout).toContain("Defaults to 150"); + complete(f); + expect(merge(f, ["--o", f.output]).status).toBe(0); + }); + + test("preserves JSON property order, ASCII escapes, and platform newlines", () => { + const f = fixture(); + writeFileSync( + f.input, + '{"preview":"é\\n","path":"old","path":"𐀀.py","area":"\\udcff"}\r', + ); + expect(make(f).status).toBe(0); + expect(readFileSync(shard(f, 1), "utf8")).toBe( + '{"preview":"\\u00e9\\n","path":"\\ud800\\udc00.py","area":"\\udcff"}' + + newline, + ); + writeFileSync( + shard(f, 1, true), + '{"reason":"é","include":false,"score":5,"area":"\\udcff","path":"𐀀.py"}', + ); + expect(merge(f).status).toBe(0); + expect(readFileSync(f.output, "utf8")).toBe( + '{"reason":"\\u00e9","include":false,"score":5,"area":"\\udcff","path":"\\ud800\\udc00.py"}' + + newline, + ); + }); + + test.each([ + ["{not json}\n", "invalid JSON"], + [ + '{"path":"a.py","area":"src","score":10,"include":true}', + "missing fields ['reason']", + ], + [ + JSON.stringify({ ...ranked(candidate("a.py")), score: true }), + "score must be an integer", + ], + [ + '{"path":"a.py","area":"src","score":5.0,"include":true,"reason":"x"}', + "score must be an integer", + ], + [ + JSON.stringify({ ...ranked(candidate("a.py")), score: 11 }), + "score must be from 1 through 10", + ], + [ + JSON.stringify({ ...ranked(candidate("a.py")), include: "true" }), + "include must be a boolean", + ], + [ + JSON.stringify({ ...ranked(candidate("a.py")), reason: "\t" }), + "reason must be a non-empty string", + ], + [ + JSON.stringify(ranked(candidate("b.py"))), + "paths do not match its input shard", + ], + [ + JSON.stringify(ranked(candidate("a.py", "other"))), + "area does not match rank input", + ], + ["", "paths do not match its input shard"], + ["\n", "blank JSONL rows are not allowed"], + [ + JSON.stringify({ ...ranked(candidate("a.py")), preview: "extra" }), + "unexpected fields ['preview']", + ], + ])( + "rejects invalid worker output %j without overwriting the merged file", + (data, message) => { + const f = fixture(); + write(f.input, [candidate("a.py")]); + expect(make(f).status).toBe(0); + writeFileSync(shard(f, 1, true), data!); + writeFileSync(f.output, "preserve existing output"); + for (const result of [validate(f), merge(f)]) { + expect(result.status).toBe(1); + expect(result.stderr).toContain(message!); + } + expect(readFileSync(f.output, "utf8")).toBe("preserve existing output"); + }, + ); + + test("rejects missing, unexpected, and duplicate worker outputs", () => { + const f = fixture(); + write(f.input, [candidate("a.py")]); + expect(make(f).status).toBe(0); + expect(validate(f).stderr).toContain( + `Rank output shard missing: ${shard(f, 1, true)}`, + ); + write(shard(f, 2, true), []); + expect(merge(f).stderr).toContain( + "missing output shards ['rank-shard-0001.output.jsonl']; unexpected output shards ['rank-shard-0002.output.jsonl']", + ); + rmSync(shard(f, 2, true)); + const row = ranked(candidate("a.py")); + write(shard(f, 1, true), [row, row]); + expect(validate(f).stderr).toContain( + "Rank output shard rank-shard-0001.output.jsonl contains duplicate paths: ['a.py']", + ); + expect(merge(f).stderr).toContain("duplicate paths"); + }); + + test.each([ + "rank-shard-x.input.jsonl", + "rank-shard-001.input.jsonl", + "rank-shard-0000.input.jsonl", + "rank-shard-00001.input.jsonl", + "rank-shard-0002.input.jsonl", + ])("rejects noncanonical shard name %s", (name) => { + const f = fixture(); + write(f.input, [candidate("a.py")]); + expect(make(f).status).toBe(0); + renameSync(shard(f, 1), join(f.directory, name)); + const result = merge(f); + expect(result.status).toBe(1); + expect(result.stderr).toContain(name); + expect(existsSync(f.output)).toBe(false); + }); + + test("sorts shard numbers numerically above four digits before checking canonical names", () => { + const f = fixture(); + write(f.input, []); + write(join(f.directory, "rank-shard-9999.input.jsonl"), []); + write(join(f.directory, "rank-shard-10000.input.jsonl"), []); + const result = merge(f); + expect(result.stderr).toContain( + "actual=['rank-shard-9999.input.jsonl', 'rank-shard-10000.input.jsonl']", + ); + }); + + test.each([ + "RANK-SHARD-0001.INPUT.JSONL", + "rank-shard-0001.İnput.jsonl", + "rank-shard-0001.ınput.jsonl", + "rank-shard-0001.input.jſonl", + "ranK-shard-0001.input.jsonl", + ])( + "matches shard globs using the platform's filename case rules: %s", + (name) => { + const f = fixture(); + write(f.input, []); + write(join(f.directory, name), []); + const creation = make(f); + const result = merge(f); + if (process.platform === "win32") { + expect(creation.status).toBe(1); + expect(creation.stderr).toContain("already contains shard files"); + expect(result.status).toBe(1); + expect(result.stderr).toContain(`invalid name: ${name}`); + } else { + expect(creation.status).toBe(0); + expect(result.status).toBe(0); + expect(readFileSync(f.output)).toHaveLength(0); + } + }, + ); + + test.each(["order", "preview", "duplicate across shards"])( + "rejects an invalid authoritative partition: %s", + (change) => { + const f = fixture(); + const rows = [candidate("a.py"), candidate("b.py")]; + write(f.input, rows); + expect(make(f, ["--max-rows", "1"]).status).toBe(0); + if (change === "order") write(f.input, [...rows].reverse()); + if (change === "preview") + write(shard(f, 1), [{ ...rows[0], preview: "changed" }]); + if (change === "duplicate across shards") write(shard(f, 2), [rows[0]!]); + complete(f); + const result = merge(f); + expect(result.status).toBe(1); + expect(result.stderr).toContain( + change === "duplicate across shards" + ? "Rank outputs contain duplicate path: a.py" + : "do not exactly partition the authoritative rank input", + ); + expect(existsSync(f.output)).toBe(false); + }, + ); + + test("rejects duplicate or malformed authoritative rows before creating a shard directory", () => { + const f = fixture(); + for (const data of [ + JSON.stringify(candidate("a.py")) + + "\n" + + JSON.stringify(candidate("a.py")), + "{}", + "\u001c\n", + "\ufeff{}", + Buffer.from([255]), + ]) { + writeFileSync(f.input, data); + expect(make(f).status).toBe(1); + expect(merge(f).status).toBe(1); + expect(existsSync(f.directory)).toBe(false); + } + }); + + test("treats a matching directory as an existing shard and reports a missing shard directory", () => { + const f = fixture(); + write(f.input, []); + expect(merge(f).stderr).toContain("Rank shard directory missing"); + mkdirSync(shard(f, 1, true), { recursive: true }); + expect(make(f).stderr).toContain("already contains shard files"); + }); + + test("uses literal dash paths, expands homes, and accepts output/input aliasing", () => { + const f = fixture(); + write(join(f.root, "-"), [candidate("a.py")]); + const env = { ...process.env, HOME: f.root, USERPROFILE: f.root }; + const result = run( + f, + "make-rank-shards", + ["--rank-input", "-", "--out-dir", "~/shards"], + env, + ); + expect(result.status).toBe(0); + complete(f); + const merged = merge({ ...f, input: "-", output: "-" }); + expect(merged.status).toBe(0); + expect(merged.stdout).toBe(`Merged 1 ranking rows into -${newline}`); + expect(read(join(f.root, "-"))).toEqual([ranked(candidate("a.py"))]); + }); + + test.skipIf(process.platform === "win32")( + "keeps symlink/.. traversal and existing output permissions", + () => { + const f = fixture(); + const outside = fixture(); + mkdirSync(join(outside.root, "nested")); + symlinkSync(join(outside.root, "nested"), join(f.root, "link")); + const directory = f.root + "/link/../shards"; + write(f.input, [candidate("a.py")]); + expect(make({ ...f, directory }).status).toBe(0); + expect( + existsSync(join(outside.root, "shards", "rank-shard-0001.input.jsonl")), + ).toBe(true); + expect(existsSync(f.directory)).toBe(false); + complete({ ...f, directory }); + const target = join(f.root, "target.jsonl"); + writeFileSync(target, "existing", { mode: 0o640 }); + symlinkSync(target, f.output); + expect(merge({ ...f, directory }).status).toBe(0); + expect(statSync(target).mode & 0o777).toBe(0o640); + expect(read(target)).toEqual([ranked(candidate("a.py"))]); + }, + ); + + test.skipIf(process.platform === "win32")( + "launcher preserves POSIX shard directory bytes", + () => { + const f = fixture(); + const pathBytes = + process.platform === "darwin" ? Buffer.from("é") : Buffer.from([255]); + const suffix = Array.from( + pathBytes, + (byte) => `\\${byte.toString(8).padStart(3, "0")}`, + ).join(""); + write(f.input, [candidate("a.py")]); + const launcher = join( + PLUGIN_ROOT, + "scripts", + "launch_codex_security_mcp", + ); + const result = spawnSync( + "sh", + [ + "-c", + `exec "$1" --helper make-rank-shards --rank-input "$2/input.jsonl" --out-dir "$2/shards-$(printf '${suffix}')"`, + "sh", + launcher, + f.root, + ], + { env: { ...process.env, CODEX_MCP_NODE_PATH: node } }, + ); + expect(result.status).toBe(0); + const directory = Buffer.concat([ + Buffer.from(f.root + "/shards-"), + pathBytes, + ]); + const input = Buffer.concat([ + directory, + Buffer.from("/rank-shard-0001.input.jsonl"), + ]); + const output = Buffer.concat([ + directory, + Buffer.from("/rank-shard-0001.output.jsonl"), + ]); + writeFileSync(output, JSON.stringify(ranked(candidate("a.py"))) + "\n"); + expect(JSON.parse(readFileSync(input, "utf8"))).toEqual( + candidate("a.py"), + ); + const merged = spawnSync( + "sh", + [ + "-c", + `exec "$1" --helper merge-rank-outputs --rank-input "$2/input.jsonl" --shard-dir "$2/shards-$(printf '${suffix}')" --out "$2/output.jsonl"`, + "sh", + launcher, + f.root, + ], + { env: { ...process.env, CODEX_MCP_NODE_PATH: node } }, + ); + expect(merged.status).toBe(0); + expect(read(f.output)).toEqual([ranked(candidate("a.py"))]); + }, + ); +}); From 314a566fa471bb8a2fed2371fcb89571ee47bfde Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Thu, 10 Sep 2026 02:47:25 +0000 Subject: [PATCH 2/4] test: remove unused shard directory preload --- sdk/typescript/tests-ts/rank-shards.test.ts | 35 ++------------------- 1 file changed, 3 insertions(+), 32 deletions(-) diff --git a/sdk/typescript/tests-ts/rank-shards.test.ts b/sdk/typescript/tests-ts/rank-shards.test.ts index af977b13e..a9495ce50 100644 --- a/sdk/typescript/tests-ts/rank-shards.test.ts +++ b/sdk/typescript/tests-ts/rank-shards.test.ts @@ -162,7 +162,7 @@ describe("rank shard helpers", () => { }); test.skipIf(process.platform === "win32")( - "preserves existing shards when Unicode entry types require metadata lookup", + "preserves existing shards alongside Unicode and raw-byte filenames", () => { const f = fixture(); write(f.input, [candidate("new.py")]); @@ -174,41 +174,12 @@ describe("rank shard helpers", () => { "unrelated undecodable file", ); const original = readFileSync(shard(f, 1)); - const preload = join(f.root, "unknown-types.cjs"); - writeFileSync( - preload, - `const fs = require("node:fs"); -const open = fs.opendirSync; -fs.opendirSync = (...args) => { - const handle = open(...args); - const read = handle.readSync.bind(handle); - handle.readSync = () => { - const entry = read(); - // Model DT_UNKNOWN lookup joining a Buffer directory with a string name. - if (entry) fs.lstatSync(require("node:path").join(args[0], entry.name)); - return entry; - }; - return handle; -}; -require("node:module").syncBuiltinESMExports(); -`, - ); - const invoke = () => - run( - f, - "make-rank-shards", - ["--rank-input", f.input, "--out-dir", f.directory], - { - ...process.env, - NODE_OPTIONS: `--require=${JSON.stringify(preload)}`, - }, - ); - const result = invoke(); + const result = make(f); expect(result.status).toBe(1); expect(result.stderr).toContain("already contains shard files"); expect(readFileSync(shard(f, 1))).toEqual(original); rmSync(shard(f, 1)); - expect(invoke().status).toBe(0); + expect(make(f).status).toBe(0); expect(read(shard(f, 1))).toEqual([candidate("new.py")]); }, ); From cadfb8ca8d1bb4c28520d1c267b3ae3cdc43d42b Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Thu, 10 Sep 2026 23:21:34 +0000 Subject: [PATCH 3/4] refactor: validate canonical shard names directly --- .../mcp-app/src/helpers/rank-shards.ts | 20 ++++--------------- .../mcp-app/src/helpers/rank-worklists.ts | 5 +---- sdk/typescript/tests-ts/rank-shards.test.ts | 14 ++++++------- 3 files changed, 11 insertions(+), 28 deletions(-) diff --git a/plugins/codex-security/mcp-app/src/helpers/rank-shards.ts b/plugins/codex-security/mcp-app/src/helpers/rank-shards.ts index 8e6cc6478..1f950a82d 100644 --- a/plugins/codex-security/mcp-app/src/helpers/rank-shards.ts +++ b/plugins/codex-security/mcp-app/src/helpers/rank-shards.ts @@ -89,29 +89,17 @@ function discoverInputShards(directory: string): string[] { } if (!isDirectory) throw new Error(`Rank shard directory missing: ${directory}`); - const numbered = shardNames(directory, "input").map((name) => { - const match = /^rank-shard-([0-9]{4,})\.input\.jsonl$/u.exec(name); - if (!match || match[0] !== name) - throw new Error(`Rank input shard has invalid name: ${name}`); - return { name, number: BigInt(match[1]!) }; - }); - numbered.sort((left, right) => - left.number === right.number - ? compare(left.name, right.name) - : left.number < right.number - ? -1 - : 1, - ); - const names = numbered.map(({ name }) => name); + const names = shardNames(directory, "input"); + const actual = new Set(names); const expected = names.map( (_, index) => `rank-shard-${String(index + 1).padStart(4, "0")}.input.jsonl`, ); - if (names.some((name, index) => name !== expected[index])) + if (expected.some((name) => !actual.has(name))) throw new Error( `Rank input shards must use contiguous canonical names; expected=${pythonRepr(expected)}; actual=${pythonRepr(names)}`, ); - return names.map((name) => childPath(directory, name)); + return expected.map((name) => childPath(directory, name)); } function validateShard(input: string, output: string): [RankRow[], RankRow[]] { diff --git a/plugins/codex-security/mcp-app/src/helpers/rank-worklists.ts b/plugins/codex-security/mcp-app/src/helpers/rank-worklists.ts index b0fe433e4..b3087035c 100644 --- a/plugins/codex-security/mcp-app/src/helpers/rank-worklists.ts +++ b/plugins/codex-security/mcp-app/src/helpers/rank-worklists.ts @@ -27,10 +27,7 @@ export function compare(left: string, right: string): number { return a.length - b.length; } -export function loadRankRows( - path: string, - selection: boolean, -): RankRow[] { +export function loadRankRows(path: string, selection: boolean): RankRow[] { const contents = decodeUtf8(readFile(path)); const lines = contents === "" ? [] : contents.split(/\r\n|[\r\n]/u); if (lines.at(-1) === "") lines.pop(); diff --git a/sdk/typescript/tests-ts/rank-shards.test.ts b/sdk/typescript/tests-ts/rank-shards.test.ts index 376e115f3..d5a78ab4f 100644 --- a/sdk/typescript/tests-ts/rank-shards.test.ts +++ b/sdk/typescript/tests-ts/rank-shards.test.ts @@ -360,9 +360,7 @@ describe("rank shard helpers", () => { const f = fixture(); write(f.input, [candidate("a.py")]); expect(make(f).status).toBe(0); - expect(validate(f).stderr).toContain( - shard(f, 1, true), - ); + expect(validate(f).stderr).toContain(shard(f, 1, true)); write(shard(f, 2, true), []); expect(merge(f).stderr).toContain( "missing output shards ['rank-shard-0001.output.jsonl']; unexpected output shards ['rank-shard-0002.output.jsonl']", @@ -393,15 +391,15 @@ describe("rank shard helpers", () => { expect(existsSync(f.output)).toBe(false); }); - test("sorts shard numbers numerically above four digits before checking canonical names", () => { + test("rejects gaps in shard names above four digits", () => { const f = fixture(); write(f.input, []); write(join(f.directory, "rank-shard-9999.input.jsonl"), []); write(join(f.directory, "rank-shard-10000.input.jsonl"), []); const result = merge(f); - expect(result.stderr).toContain( - "actual=['rank-shard-9999.input.jsonl', 'rank-shard-10000.input.jsonl']", - ); + expect(result.status).toBe(1); + expect(result.stderr).toContain("contiguous canonical names"); + expect(existsSync(f.output)).toBe(false); }); test.each([ @@ -422,7 +420,7 @@ describe("rank shard helpers", () => { expect(creation.status).toBe(1); expect(creation.stderr).toContain("already contains shard files"); expect(result.status).toBe(1); - expect(result.stderr).toContain(`invalid name: ${name}`); + expect(result.stderr).toContain("contiguous canonical names"); } else { expect(creation.status).toBe(0); expect(result.status).toBe(0); From 8b0f697a50cbbb7c83e5c13dabe8c2c929f252be Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Thu, 10 Sep 2026 23:23:48 +0000 Subject: [PATCH 4/4] test: keep Windows shard rejection proof aligned --- .../codex-security/native/examples/windows-wide-launcher.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/codex-security/native/examples/windows-wide-launcher.rs b/plugins/codex-security/native/examples/windows-wide-launcher.rs index 139c10f0d..8ec31aa5e 100644 --- a/plugins/codex-security/native/examples/windows-wide-launcher.rs +++ b/plugins/codex-security/native/examples/windows-wide-launcher.rs @@ -517,7 +517,9 @@ fn main() -> std::io::Result<()> { let malformed = shard_command("merge-rank-outputs", &merge_args)?; if malformed.status.code() != Some(1) || !String::from_utf8_lossy(&malformed.stderr) - .contains("Rank input shard has invalid name: rank-shard-\\udfff.input.jsonl") + .contains("Rank input shards must use contiguous canonical names") + || !String::from_utf8_lossy(&malformed.stderr) + .contains("rank-shard-\\udfff.input.jsonl") || fs::read(&merged)? != output_rows.concat().as_bytes() || fs::read(&shard_sentinel)? != b"replacement shard sentinel" || fs::read(&merge_sentinel)? != b"replacement merge sentinel"