You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
refactor(targets)!: run inventory and ranking helpers in Node - #1432
Run scoped inventory and diff-ranking input generation through the bundled Node helpers so these preparation steps no longer require their former Python entrypoints. Preserve the existing artifact formats, raw paths, and custom-plugin compatibility.
Changes
Move inventory generation, scoped source enumeration, and diff previews into the plugin-owned Node runtime; update SDK preparation, MCP callers, instructions, packaging, and tests together.
Retire generate_in_scope_files.py, generate_rank_input.py, and rank_preview.py. Keep the retired repository-ranking command unavailable and retain the supported custom-plugin Python path.
Preserve destination permissions, subprocess errors, and inherited settings for fresh and resumed Deep Scan workers. Cover ambiguous Git revision filenames and unreadable descendants in both tracked-directory replacement and tool-free inventory enumeration.
Keep the selected Codex runtime’s bundled ripgrep available for Windows installations inside the scanned project, while preserving Git checks and excluding other repository executables.
Restore the existing source-metadata exclusion for local candidate sources, including deleted tracked metadata files; verify rejection through the real candidate writer while keeping ordinary deleted sources valid.
Testing
Merged main at fe9d77e1c3d9; this refresh changes only the two Dockerfile base-image references to Node 26.12.1. The merge is clean, git diff --check passes, and every tracked entry outside Dockerfile is identical to the previously published head. The validation below is historical evidence for its named revisions; runtime tests and image builds were not repeated for this Dockerfile-only refresh. Fresh exact-head hosted CI remains required.
Local validation at e04a4571a256af239d9ccc1795c4a828980888be, including main at 5f521759e7c6ff8c3fbd5e81fd1a3567ac801b59, passed:
68 workflow/report tests, 27 native-configuration tests, and 6 package-smoke-timeout tests; no failures.
SDK CI-helper compilation, SDK/MCP type checks, formatting for the refreshed files, Actionlint syntax/expression checks, and git diff --check.
These Linux checks used Node 22.23.3 and Bun 1.3.14, seed 12345. Existing dependencies and the generated plugin/native bundle were reused after matching dependency bytes, source/build inputs and artifact hashes. ShellCheck was unavailable and disabled. No fresh plugin/native build, full SDK suite, installed-package smoke run, or cross-platform pass is claimed; fresh exact-head hosted CI remains required.
Earlier inventory/ranking helpers and worker configuration validation records carried by 603f1859942e07e4e0808cc0d5f99ac802612213 remain historical evidence for their named revisions. Their broader results are retained in the prior validation record and are not claimed as fresh execution for this main refresh.
Risk and rollout
Direct callers of the retired Python inventory entrypoints must migrate to the corresponding bundled helper commands. SDK and MCP callers use them automatically; artifact formats remain compatible. The prerequisite Node runtime work is already on main. Earlier revisions passed Windows and macOS hosted CI; the preceding refresh had only the local Linux validation above.
Public disclosure review
No customer, partner, prospect, or user identities, data, or identifying details are included.
No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.
mldangelo-oai
changed the title
refactor(plugin)!: move inventory and ranking to Node
refactor(targets)!: move inventory and ranking to Node
Oct 8, 2026
mldangelo-oai
added
area:integrations
SDK/API surfaces, MCP integrations, hooks, and external application connections.
area:targets
Repository and path targets, Git discovery, scope, inventories, and snapshots.
breaking-change
Breaking change called out in generated release notes
platform:windows
Windows, PowerShell, Windows paths, or Windows ACLs
labels
Oct 8, 2026
The fallback eagerly materializes the entire tree before checking the cheap ancestor conditions. In a Git checkout where both Git and ripgrep are unavailable, the .git marker already requires failure, but this first walks the repository (including .git/objects) and retains every path, which can cause a long delay or excessive memory use. Check ancestor ignore markers before calling walk(), and only walk when descendant ignore files still need inspection.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Addressed the traversal-order note in this review: the missing-tool fallback checks ancestor Git and ignore markers before enumerating descendants. Regressions with unreadable subtrees verify the established refusal message and preserve the existing output.
This now runs inventory generation inside the MCP server process, but working-tree diff generation calls synchronous sampleFile reads over every byte of each changed text file (and uses other synchronous traversal/writes). A large local-patch file blocks the MCP event loop, whereas the previous asynchronous child process kept the server responsive. Run the Node helper out of process here or make the inventory filesystem path asynchronous/yielding.
Prune ignored subtrees and .git directories during traversal
The fallback eagerly expands walk(scope) before checking the discovered ignore files. A descendant .gitignore, .ignore, or .rgignore therefore cannot stop traversal of its ignored (possibly huge or unreadable) subtree, and nested .git directories are traversed even though they are excluded from inventories. Detect these markers and prune .git before descending rather than inspecting the completed walk.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the changes since the previous reviewed head in two passes, using exact source at 165d355e5e43 and prior review evidence. Unchanged portions of the large branch were not exhaustively re-audited.
Approved with these considerations still open:
P2: Use the same ripgrep selection order for ordinary and raw Windows inputs. Raw UTF-16 switches from rg.com-first to rg.exe-first, so a filename or environment value can change which installed binary runs. Keep [rg.com, rg.exe] in both branches. plugins/codex-security/mcp-app/src/helpers/inventory-git.ts:102
Validation: CI-helper TypeScript compilation and plugin bundling passed locally; native binaries came from CI with matching compiled source inputs. Focused SDK tests passed: diff-rank-input.test.ts. The full cross-platform suite was not run locally; Windows execution remains unverified here.
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the actual diff against #1431 at 8e2fd6c223e157a6a45190b7401d6986ea4764f3 in three independent passes, including the new inventory/path tests and the discovery, reducer, and resumed-worker launch assertions. The port preserves explicit scope, literal path transport, resolved revision IDs, child settings, and compatibility with the documented custom-plugin path. The earlier Windows ripgrep encoding concern is inherited behavior rather than a regression introduced here.
No blocking findings. Static review only; native addons, Windows commands, generators, and integration suites were not executed.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the migration. Owner-level merge-safety review of fb1e900d2a8e against the #1431 base 0bb8955403b7 found no blocking regression. The Python inventory entrypoints are intentionally removed; the built Node helpers preserve the artifact formats and explicit custom-plugin Python compatibility. One nonblocking suggestion is to keep the retired repository-ranking command out of the dispatcher.
The new helper produced byte-identical output to the exact-base Python entrypoints in 13 practical cases. Types, formatting, plugin build and all five portable checks pass; local SDK inventory tests passed 161 cases with 32 platform skips and 25 environmental fixture failures. Selected hosted Linux/Windows suites and native Windows proofs pass. No tracked files were changed.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the migration and for removing the retired repository-ranking command. Owner-level merge-safety review of db321bca51f1 found no blocking regression. The Node helpers preserve scoped inventories, diff previews, and the explicit custom-plugin Python fallback.
Fresh checks passed 29 inventory cases, 10 Python compatibility cases, 14 MCP path/candidate cases, three SDK prompt/integration cases, types, formatting and all five portable checks. Thirteen practical cases produced byte-identical output to the exact-base Python helpers. The current CI MCP report has 533 passing cases and four skips, including inventory, Deep worker execution and MCP smoke; its tested merge tree exactly matches this head. Full local platform suites were not repeated because their previously established ownership/Git fixture restrictions remain.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the inventory fixes. Owner-level review of b501598a3266 found no material issue. I checked the unreadable-directory fallback and revision/path disambiguation, then verified all 46 feature patches through the latest main integration. Fresh checks passed the three targeted inventory cases, plugin build, types, formatting and all portable checks. The immediately preceding exact-input checks passed 42 inventory and seven Python cases; one tool-free fallback fixture remains blocked by the test environment’s enclosing Git directory, and the separate strict/tolerant walker control passes. Prior full inventory/parity evidence remains applicable where inputs are unchanged.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the Windows inventory fix. Reviewed 5d6ca8198e1c against dd42caa, including the selected Codex bundled-ripgrep identity exception and both inventory entrypoints. The exception is limited to the selected runtime's actual bundled tool; the remaining repository-tool exclusions stay intact. No material issue found.
Validation: plugin build, MCP types and changed-file formatting passed. The inventory suite passed 129 cases, skipped 23 and failed 22; all 22 failures reproduced in controls on the preceding reviewed head. The four new Windows cases are among the skips, so native Windows execution remains unverified here. Other feature, dependency and instruction inputs are unchanged from the prior full review; unaffected validation is reused with its recorded limits.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for completing the Node inventory migration and restoring the local metadata exclusion. Reviewed the current 47-file change at 25a7ed5, including custom-plugin fallback, raw paths, subprocess diagnostics, scope boundaries and worker settings. The deleted .svn source issue is fixed; no actionable issue remains.
Fresh validation passed builds, types, formatting, portable checks, 13 byte-for-byte comparisons with the base Python helpers, the real candidate-writer regression and 15 Python target cases. Focused SDK checks passed 140 cases; 25 platform cases were skipped and 19 cases hit established Git/ownership fixture restrictions. MCP path/candidate checks passed 14 cases with two skips. Full platform and package validation remain incomplete locally.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. The Node inventory/helper migration remains consistent with its selected-scope and raw-path contracts. I reviewed the current main integrations, manifest and caller changes, and the small Python classifier inlining; no remaining material issue found.
Builds, types, formatting and required portable checks passed, as did 137 inventory/ranking cases, three API scope cases, 15 MCP cases and eight base/head binary-sampling controls. The 17 known local inventory fixture failures recur, and broader Python fixtures remain mixed under the directory-ownership/Git-ancestry constraints. Current hosted reports pass 2,010 affected cases with 235 skips and no failures; relevant helper/runtime/test inputs match, although the tested merge includes later main changes. This is not a full local or exact-head platform-suite pass.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. The inventory/helper migration remains consistent with the selected-scope and raw-path contracts. I reviewed the five changed files and the incoming usage-observer, target-classification and runtime-cleanup integrations; no remaining material issue found.
Fresh checks passed for 12 saved-excerpt cases, three API scope cases, all nine portable source checks, builds, types and formatting. The broad component run was mixed (16 passed, 48 failed), with representative failures at the existing directory-ownership and Git-ancestry fixture restrictions; I am not claiming that suite passed. The earlier inventory/helper evidence remains applicable to the proven unchanged inputs.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. No material issue found in this owner-level review. The latest delta moves help-smoke coverage from the retired Python entrypoints to their three Node replacements; production code is unchanged from the reviewed 5eb62ac head. The inventory migration and custom-plugin compatibility remain sound.
At this head, 14 Python help/import cases and both focused Node helper cases passed. All five required portable commands passed, including nine source-checker test cases, as did the plugin build, types and formatting. The preceding inventory/API/worker validation remains applicable to the unchanged implementation. Full SDK and platform validation is still qualified historical evidence.
…es (#2109)
The extension allowlist that silently dropped `.cgi`, `.pl`, `.pm`, and `.t`
files was removed in #1347, and #1432 moved inventory generation to Node. The
move left no coverage for the reported Perl/mixed-encoding scenario: the
Python `source_cases` matrix that carried unfamiliar and extensionless sources
is no longer referenced, so a future suffix filter would regress silently.
Add a diff-inventory regression that includes `.cgi`, `.pl`, `.pm`, `.t`, an
extensionless Perl shebang script, and a file stored as EUC-JP rather than
UTF-8. A `.pm` containing NUL bytes stays excluded, pinning selection to file
content rather than suffix.
Refs #2078.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
breaking-changeBreaking change called out in generated release notesplatform:windowsWindows, PowerShell, Windows paths, or Windows ACLs
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
zcrab-oai
left a comment
alandelong-oai
left a comment
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Summary
Run scoped inventory and diff-ranking input generation through the bundled Node helpers so these preparation steps no longer require their former Python entrypoints. Preserve the existing artifact formats, raw paths, and custom-plugin compatibility.
Changes
generate_in_scope_files.py,generate_rank_input.py, andrank_preview.py. Keep the retired repository-ranking command unavailable and retain the supported custom-plugin Python path.Testing
Merged
mainatfe9d77e1c3d9; this refresh changes only the two Dockerfile base-image references to Node 26.12.1. The merge is clean,git diff --checkpasses, and every tracked entry outside Dockerfile is identical to the previously published head. The validation below is historical evidence for its named revisions; runtime tests and image builds were not repeated for this Dockerfile-only refresh. Fresh exact-head hosted CI remains required.Local validation at
e04a4571a256af239d9ccc1795c4a828980888be, includingmainat5f521759e7c6ff8c3fbd5e81fd1a3567ac801b59, passed:git diff --check.These Linux checks used Node 22.23.3 and Bun 1.3.14, seed
12345. Existing dependencies and the generated plugin/native bundle were reused after matching dependency bytes, source/build inputs and artifact hashes. ShellCheck was unavailable and disabled. No fresh plugin/native build, full SDK suite, installed-package smoke run, or cross-platform pass is claimed; fresh exact-head hosted CI remains required.Earlier inventory/ranking helpers and worker configuration validation records carried by
603f1859942e07e4e0808cc0d5f99ac802612213remain historical evidence for their named revisions. Their broader results are retained in the prior validation record and are not claimed as fresh execution for this main refresh.Risk and rollout
Direct callers of the retired Python inventory entrypoints must migrate to the corresponding bundled helper commands. SDK and MCP callers use them automatically; artifact formats remain compatible. The prerequisite Node runtime work is already on main. Earlier revisions passed Windows and macOS hosted CI; the preceding refresh had only the local Linux validation above.
Public disclosure review