Skip to content

refactor(plugin): migrate repository scope binding to TypeScript - #843

Open
kmbroai wants to merge 3 commits into
dev/kyleb/python-free-rank-poolfrom
dev/kyleb/python-free-repo-scope-binding
Open

kmbroai wants to merge 3 commits into
dev/kyleb/python-free-rank-poolfrom
dev/kyleb/python-free-repo-scope-binding

Conversation

@kmbroai

@kmbroai kmbroai commented Sep 9, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Move repository scope binding to the bundled TypeScript helper and use it in generated scan instructions.

Changes

  • Preserve manifest and coverage updates, JSON formatting, and scope validation.
  • Keep the shared serializer required by canonical artifacts and binding manifests in the remaining migration.
  • Update SDK and skill invocations, including Windows CMD expansion; remove the Python command and tests.
  • Integrate every preceding PR, including the current Windows file-operation fixes, into the combined stack tip.

Testing

  • Combined stack tip ba12023: both full SDK runs passed 2,656 tests with 50 skips and zero failures (seeds 12345 and 4201856736).
  • The six affected helper suites passed 213 tests with four Windows-only skips; all 23 MCP test processes passed.
  • CI compilation, plugin build, types, formatting, Ruff, portable source checks, and nine source-checker tests passed.
  • Rust formatting and cross-target Clippy for the Windows x64 native proof passed. Local execution was Linux; hosted CI supplies platform runtime coverage.

Risk and rollout

Stacked on ranking pools. Invocation becomes launch_codex_security_mcp[.cmd] --helper bind-repo-scopes with the existing required scopes-file, manifest, and coverage options. The combined tip includes all current parent fixes. Actual Windows/macOS execution is checked by hosted CI.

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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review ✅ Completed 2026-09-11T00:08:24.300264Z 5bd1d19 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kmbroai
kmbroai force-pushed the dev/kyleb/python-free-repo-scope-binding branch from 224e8d0 to 55a6339 Compare September 9, 2026 03:04
@kmbroai
kmbroai added this pull request to stack #855 September 9, 2026 22:16
@mldangelo-oai

Copy link
Copy Markdown
Collaborator

Whole-stack QA against main

Completed the combined-stack comparison with GPT-6 Astra at ultra before starting individual PR reviews. Baseline: 6ecb9db5180e33c31d1d3f0f3dda3fe966463968. The integration checkout includes all eight current PR heads, including #836 at 06112a516431a9e12de644e8621b0176946da0a5 and #843 at 55a6339212290726d08bb708634769097e6153fd.

No stack-only functional regression was observed in the covered Linux contracts. This is not an all-platform pass.

Verification Main Combined stack
Full SDK CI inventory, all three Linux shards, seed 12345 2,651 pass / 47 skip / 1 fail 2,847 pass / 51 skip / 1 fail
Full Python suite 1,088 pass / 8 skip; 109 subtests pass 986 pass / 6 skip; 109 subtests pass
Full MCP suite 23/23 file test processes pass 23/23 pass
Required Ruff, portable source, build, type, format and source-consistency checks Pass Pass
Packed and installed package on Node 22.13, 24.0 and 26.0 Pass on all three Pass on all three
Linux native build and behavioral proofs Pass Pass

The one SDK failure is the unchanged malformed-Git-config case in security-policy.test.ts. It reproduces on both revisions with the same Git build and passes on both in a focused control using another installed Git version. The full suite therefore retains that baseline failure.

Additional checks through built helpers covered 209 paired core scenarios and 155 paired rank/pool/scope scenarios, including a ten-stage shard-to-scope workflow and independently checked artifact bytes, modes, ordering and receipt hashes. All 155 rank scenarios match. Core comparisons match ordinary behavior; the two acceptance differences are 4,301-digit integer inputs rejected by Python's default conversion limit. Diagnostic wording also differs. I do not see a reason to copy those incidental Python limits or tracebacks into Node. Representative core and complete rank compositions also pass through the installed package.

Integration and platform gap

The current stack tip does not contain #836's latest follow-up commits. For this QA I merged the current #843 and #836 heads onto main in an isolated checkout. Before claiming the combined stack is ready across platforms, restack to include those fixes and run CI on that result.

The original pinned PR heads have successful Windows/macOS, native-target and PowerShell CI evidence. Those runs test their original trees; none tests this combined integration tree. Local execution here establishes Linux behavior only. Source-matched CI native payloads passed the distribution compatibility checks; ordinary workstation-built payloads exceeded the GLIBC distribution floor on both revisions.

The stack adds a net 1,581 production lines and 2,078 test/proof lines. The individual reviews are examining concrete deletions and simpler implementations, particularly the JSON/schema/path adapters, while preserving real artifact and filesystem contracts. No implementation changes or formal review disposition are included in this QA comment.

Comment thread plugins/codex-security/mcp-app/src/helpers/python-json.ts
@mldangelo-oai

Copy link
Copy Markdown
Collaborator

Reviewed 55a6339212290726d08bb708634769097e6153fd against its #842 base using GPT-6 Astra at ultra effort, with the PR deslop and critical review passes.

The scope-binding migration is useful and correctly connected to the SDK workflow. I found no introduced correctness or security defect. One nonblocking reduction removes 65 formatted lines by using compact JSON for unsealed documents and diagnostics, while preserving exact numeric values and final sealing behavior.

Verification beyond the complete-stack comparison with main:

  • 42 focused Bun tests passed, including execution of the emitted POSIX shell command; 28 retained Python generator tests passed. Builds, types, Ruff and portable source checks passed.
  • 60 Python-base/head/variant binding-to-finalizer comparisons on Node 20 and 22: 40 valid cases produced identical sealed files; 20 invalid cases failed without finalizer writes.
  • 918 assessment mutations per runtime on Node 20, 22, 24, 26 and Bun preserved schema acceptance.
  • Twelve real helper compositions on Node 20/22/24/26 covered 204 calls and preserved ranking artifacts, raw scope paths, symlinks and file modes. All 24 worker receipt hashes matched independent recomputation.

Pre-seal JSON presentation and diagnostics intentionally change in the proposed simplification. The shared argument-parser alternative is attributed to #839, where that code originated, with its CLI compatibility changes stated separately.

Local OS execution was Linux. Exact-head CI supplies separate Windows, PowerShell, macOS and native-runtime evidence. The alternatives need platform CI when applied. Restack the later PRs onto the latest #836 and rerun that CI for the combined stack before merging.

kmbroai commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator Author

Addressed both the PR review and the whole-stack QA. The shared serializer remains because the remaining migration uses its canonical representation for sealed artifacts and binding manifests. The stack is now restacked: this tip contains every current parent, including the latest Windows file-operation fixes and the shard/pool cleanup. All inline discussions across the eight PRs have a fix or a rationale.

Updated head: 5bd1d1909ad99d2749fe8d3ec49a9a144ae2bac0. Hosted CI passes on this head, including Linux/macOS/Windows tests, native proofs, installed-package checks, plugin source contracts, types, and formatting. Codex Security Review also passes. The Windows jobs passed on an unchanged-commit retry after npm installation and a legacy Python subprocess timed out.

This closes the combined-stack platform gap: 5bd1d1909ad99d2749fe8d3ec49a9a144ae2bac0 contains every current parent, and its platform CI is green. Its source tree is identical to the locally tested ba12023f7f835bdc58f7200d5997670cda2f08a0 tree. Both full SDK runs passed 2,656 tests (50 skipped, zero failures; seeds 12345 and 4201856736). The 213 focused helper tests, all 23 MCP test processes, portable source checks, Ruff, and the Windows proof compile check also passed.

All 14 inline threads across the eight PRs now have a fix or an explicit rationale and are resolved; all nine general review comments are addressed. The PRs remain open for review approval.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants