feat(plugin): prototype TypeScript candidate normalization - #643
feat(plugin): prototype TypeScript candidate normalization#643mldangelo-oai wants to merge 3 commits into
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2a088a5bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (inputs.includes(output)) | ||
| throw new Error("--out: must not also be an input"); | ||
| if (output === scopePath) { | ||
| throw new Error("--out: must not replace --in-scope-files"); |
There was a problem hiding this comment.
On Windows, path identity is case-insensitive, but these string comparisons are case-sensitive and resolveAllowMissing() preserves the caller's spelling. If an existing input or scope file is passed as --out with different drive, directory, or filename casing, the check is bypassed and the final rename replaces that protected file; Python's Path comparison rejects the same invocation. Compare canonical Windows path keys or otherwise use case-insensitive path identity here.
AGENTS.md reference: sdk/typescript/AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
| function positiveLine(value: unknown, field: string): number { | ||
| if (typeof value !== "number" || !Number.isInteger(value) || value < 1) { | ||
| throw new Error(`${field}: expected a positive integer`); | ||
| } | ||
| return value; |
There was a problem hiding this comment.
When model output encodes a line as 1.0 or 1e0, JSON.parse produces the number 1, so this check accepts it and writes a candidate; the Python oracle parses those tokens as floats and rejects them because they are not int values. This breaks the documented rejected-input parity during migration and can cause previously invalid candidate rows to enter scan results, so the JSON token type must be preserved or validated before coercion.
AGENTS.md reference: AGENTS.md:L21-L24
Useful? React with 👍 / 👎.
| const entrypoint = process.argv[1]; | ||
| if ( | ||
| entrypoint !== undefined && | ||
| import.meta.url === pathToFileURL(resolve(entrypoint)).href | ||
| ) { |
There was a problem hiding this comment.
When this script or any parent directory is invoked through a filesystem symlink, Node resolves import.meta.url to the real module while process.argv[1] retains the symlink spelling, so this condition is false and the process exits successfully without reading candidates or producing output. The Python helper works through the same symlink, so canonicalize both paths before comparing them to preserve executable behavior.
AGENTS.md reference: AGENTS.md:L21-L24
Useful? React with 👍 / 👎.
| async function runCli(): Promise<void> { | ||
| if (process.argv.slice(2).some(isHelpArgument)) { | ||
| console.log(HELP); |
There was a problem hiding this comment.
If --help or -h follows an option whose value is missing, such as --out --help or --input --help, this pre-scan prints help and exits successfully. The Python CLI instead reports the missing value and exits with status 2, so malformed automation can be silently treated as successful during migration; process help in argument order rather than bypassing parsing whenever it appears.
AGENTS.md reference: AGENTS.md:L28-L31
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40df333b76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (value === undefined || value.startsWith("-")) { | ||
| throw new Error(`${option}: expected a value`); |
There was a problem hiding this comment.
When a scalar path is supplied as a separate token that resembles a negative number, such as --out -1, this check rejects it as a missing value. The Python oracle’s argparse parser accepts -1 here—I checked --out -1 --help, which prints help and exits 0—so scripts using numeric-looking filenames fail after switching to this helper unless they rewrite the argument using =. Preserve the existing accepted path syntax rather than rejecting every dash-prefixed token.
AGENTS.md reference: AGENTS.md:L28-L31
Useful? React with 👍 / 👎.
| function expandUser(value: string): string { | ||
| if (value === "~") return homedir(); | ||
| if (value.startsWith(`~${sep}`) || (sep === "\\" && value.startsWith("~/"))) { | ||
| return join(homedir(), value.slice(2)); | ||
| } | ||
| return value; |
There was a problem hiding this comment.
On Unix, quoted or programmatically supplied paths such as ~alice/repo are expanded by every Path.expanduser() call in the Python oracle, but this implementation recognizes only the current-user forms ~ and ~/.... Consequently, any input, repository root, scope file, or output under another user’s accessible home is instead resolved relative to the current working directory and fails or addresses the wrong path, changing an existing accepted CLI value during migration.
AGENTS.md reference: AGENTS.md:L28-L31
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5af166bb68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
|
|
||
| const CWE = /^CWE-(\d+)$/iu; |
There was a problem hiding this comment.
When a candidate contains a CWE such as CWE-١ or CWE-１２, the Python oracle accepts it because Python's \d matches Unicode decimal digits and int() normalizes them, while JavaScript's \d remains ASCII-only even with the u flag, so this prototype rejects an input the existing normalizer converts to CWE-1 or CWE-12. Use Unicode decimal-digit matching and normalization to preserve accepted-input and output parity.
AGENTS.md reference: AGENTS.md:L21-L22
Useful? React with 👍 / 👎.
1 participant
Summary
Add a side-by-side TypeScript candidate normalizer with Python as the executable oracle. Python remains the production entrypoint.
Changes
Testing
Final affected suites, seed 643: 165 passed, 14 platform skips. The token, symlink, help-order and CR-only regressions were reproduced before their fixes.
Generated helper under Node 22.0.0: 21 passed, two Windows-only skips across all three differential suites. A separate expanded property run used 32 cases per property plus explicit invalid cases.
Types, generated models/helper checks, formatting and build passed.
Real Codex upgrades from cache versions 0.1.37 and 0.1.60 replaced a stale helper and executed the installed normalizer.
Fresh 283-entry package check and full installed smoke passed with 119 plugin files, SDK import, NodeNext consumer, CLI, credential locking and a nested worker.
Native Windows and Linux, the full suite and live model execution were not rerun. Current-head CI is left for a separate pass; the old Windows failures are not presented as a green current-head run.
Final main refresh (
fd98a90): package 0.1.21 includes the MCP launcher-permission fix; SDK implementation and bundled payload are unchanged. Types/model generation, formatting, build, 49 focused tests (2 platform skips), static artifact verification and full installed-package smoke passed, including MCP initialization. CI was not awaited.Risk and rollout
No production cutover or public CLI change. The prototype remains bundled alongside Python for parity testing. Windows regressions are included but require native CI verification. The cleanup removes nine source/generated lines; fixes and regression coverage add 72 net lines overall. Existing review discussions remain open; no approval or PR merge is included.
Public disclosure review
Historical commit contact metadata and restricted links in automated comments remain, so the second attestation is unchecked. This update uses synthetic fixtures and a GitHub noreply commit identity; other authors' comments are unchanged.