[PR #6293] [MERGED] fix(ci): isolate packed Python syntax checks from ambient imports #7260

New issue

Closed

opened 2026-11-01 21:46:34 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:34 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6293
Author: @gensokyo-dev
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/packed-python-import-isolation-20261022

📝 Commits (1)

e65aaa2 fix(ci): isolate packed Python syntax checks from ambient imports

📊 Changes

3 files changed (+77 additions, -0 deletions)

View changed files

📝 docs/testing.md (+7 -0)
📝 scripts/verify-packed-bridge.mjs (+4 -0)
📝 tests/boundary/process/packedBridgeVerification.test.ts (+66 -0)

📄 Description

Summary

The packed bridge verifier could import a caller-local ast.py or startup hook and accept malformed Python instead of checking it with the standard parser. On main 002e1b68c, the same invalid archive rejects with SyntaxError in a clean directory but passes with a shadow module; that module also writes a controlled execution marker.

Run the existing python3 command with -I -S, isolating module lookup and disabling site initialization. Archive paths, member checks, output/error semantics and deadlines remain. Python fixture archive writers use the same isolation. See Python's documented options.

Scope and evidence

This is build/package verification only, with no CLI/MCP, provider, target authority, installation or generated-metadata contract change. No new maintainer scope decision is needed for this routine correctness fix. No separate issue: this PR contains the minimal report and reproduction.

Executed red-before controls: all three new CWD, PYTHONPATH, and sitecustomize cases accept invalid source on the baseline. The clean control rejects the same bytes; the baseline shadow module's execution marker exists.
Executed green-after: all 23 packed-bridge tests pass, including the original 20 malformed-archive, member, link, syntax, read-only and owned-root cases. The three new cases reject with SyntaxError and leave the execution marker absent. Environment/cwd changes live in isolated child processes.
Executed: 41 focused consumer tests in three files, full npm run check, subsequent check:fast, and docs:check pass. Node 24.18.0, npm 11.16.0, Python 3.11.2; four-CPU affinity, serial Turbo, two checker threads. Consumer task: 4.591 s wall, 6.271 s CPU, 558.7 MiB sampled family RSS; full static task: 28.502 s, 55.760 s CPU, 2,464.2 MiB RSS.
Equivalent local task (10 valid + 10 invalid real archive checks, fresh Node/Python processes, warm filesystem): before/after alternating wall samples 0.770/0.706 and 0.760/0.708 s; CPU 0.812/0.741 and 0.805/0.741 s; RSS 84.3/86.2 and 84.3/84.6 MiB. Subsecond samples do not establish CI/Windows acceleration. 100 ms descendant-RSS sampling misses short children and may double-count shared pages; no memory improvement claim. Agent tokens and Windows CPU/memory remain unmeasured.
Executed on exact head e65aaa24a: full CI passes all 40 selected jobs, with one intentional source-test skip; portable coverage reports 25,476 passed and 201 skipped tests. Canonical PR CI also passes every selected lane. Real Windows curated/package, four-platform package smoke, Apple and other selected provider lanes pass. Greptile reviewed exact e65aaa24a at 5/5 with no findings. Codex review could not run because its service quota is exhausted.
Updated-main integration at 66aacc6fb applies cleanly; static checking and direct consumer/changed evaluation tests pass. Tested integration tree: e11aee29418269fc7b33868d37ef0f287fe11bd4. Merged normally as 21f9af9d93dacfe06848298ffae49112977a90ff (no bypass). Its actual tree 6f8b26d3436b1e719f09d5254fe92af450081b0b matches the reviewed merge result; the final independent regex-configuration delta did not touch the Python boundary. Full validation of that exact merged commit passes on attempt 1: all 40 selected jobs succeed, with one intentional source-test skip; 25,824 portable tests pass and 201 are named skips (1,982 files pass, 25 skipped). Real Windows package/23-case bridge checks and Node 24/22 native suites pass. The ordinary push run was automatically coalesced after later main commits; this fixed-ref full run verifies the exact merged SHA and uses read-only cache permissions. Later main commits are outside this pinned checkpoint.
This does not establish or claim a fix for the separate historical Windows first-Python-call 30-second timeout. Python executable/installation integrity remains a prerequisite; this is not a sandbox for an arbitrary interpreter.

Author readiness

Human owner: @gensokyo-dev

Declarations entered under the user's explicit delegation; independent human review was not observed in this session.

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6293 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/packed-python-import-isolation-20261022` --- ### 📝 Commits (1) - [`e65aaa2`](https://github.com/morluto/rea/commit/e65aaa24ab93101348ece4b676239a93e2853f5c) fix(ci): isolate packed Python syntax checks from ambient imports ### 📊 Changes **3 files changed** (+77 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `docs/testing.md` (+7 -0) 📝 `scripts/verify-packed-bridge.mjs` (+4 -0) 📝 `tests/boundary/process/packedBridgeVerification.test.ts` (+66 -0) </details> ### 📄 Description ## Summary The packed bridge verifier could import a caller-local `ast.py` or startup hook and accept malformed Python instead of checking it with the standard parser. On main `002e1b68c`, the same invalid archive rejects with `SyntaxError` in a clean directory but passes with a shadow module; that module also writes a controlled execution marker. Run the existing `python3` command with `-I -S`, isolating module lookup and disabling site initialization. Archive paths, member checks, output/error semantics and deadlines remain. Python fixture archive writers use the same isolation. See [Python's documented options](https://docs.python.org/3/using/cmdline.html#cmdoption-I). ## Scope and evidence This is build/package verification only, with no CLI/MCP, provider, target authority, installation or generated-metadata contract change. No new maintainer scope decision is needed for this routine correctness fix. No separate issue: this PR contains the minimal report and reproduction. - Executed red-before controls: all three new CWD, `PYTHONPATH`, and `sitecustomize` cases accept invalid source on the baseline. The clean control rejects the same bytes; the baseline shadow module's execution marker exists. - Executed green-after: all 23 packed-bridge tests pass, including the original 20 malformed-archive, member, link, syntax, read-only and owned-root cases. The three new cases reject with `SyntaxError` and leave the execution marker absent. Environment/cwd changes live in isolated child processes. - Executed: 41 focused consumer tests in three files, full `npm run check`, subsequent `check:fast`, and `docs:check` pass. Node 24.18.0, npm 11.16.0, Python 3.11.2; four-CPU affinity, serial Turbo, two checker threads. Consumer task: 4.591 s wall, 6.271 s CPU, 558.7 MiB sampled family RSS; full static task: 28.502 s, 55.760 s CPU, 2,464.2 MiB RSS. - Equivalent local task (10 valid + 10 invalid real archive checks, fresh Node/Python processes, warm filesystem): before/after alternating wall samples 0.770/0.706 and 0.760/0.708 s; CPU 0.812/0.741 and 0.805/0.741 s; RSS 84.3/86.2 and 84.3/84.6 MiB. Subsecond samples do not establish CI/Windows acceleration. 100 ms descendant-RSS sampling misses short children and may double-count shared pages; no memory improvement claim. Agent tokens and Windows CPU/memory remain unmeasured. - Executed on exact head e65aaa24a: [full CI](https://github.com/morluto/rea/actions/runs/39750508964) passes all 40 selected jobs, with one intentional source-test skip; portable coverage reports 25,476 passed and 201 skipped tests. [Canonical PR CI](https://github.com/morluto/rea/actions/runs/39750246640) also passes every selected lane. Real Windows curated/package, four-platform package smoke, Apple and other selected provider lanes pass. Greptile reviewed exact e65aaa24a at 5/5 with no findings. Codex review could not run because its service quota is exhausted. - Updated-main integration at 66aacc6fb applies cleanly; static checking and direct consumer/changed evaluation tests pass. Tested integration tree: e11aee29418269fc7b33868d37ef0f287fe11bd4. Merged normally as `21f9af9d93dacfe06848298ffae49112977a90ff` (no bypass). Its actual tree `6f8b26d3436b1e719f09d5254fe92af450081b0b` matches the reviewed merge result; the final independent regex-configuration delta did not touch the Python boundary. [Full validation of that exact merged commit](https://github.com/morluto/rea/actions/runs/39754027495) passes on attempt 1: all 40 selected jobs succeed, with one intentional source-test skip; 25,824 portable tests pass and 201 are named skips (1,982 files pass, 25 skipped). Real Windows package/23-case bridge checks and Node 24/22 native suites pass. The ordinary push run was automatically coalesced after later main commits; this fixed-ref full run verifies the exact merged SHA and uses read-only cache permissions. Later main commits are outside this pinned checkpoint. - This does not establish or claim a fix for the separate historical Windows first-Python-call 30-second timeout. Python executable/installation integrity remains a prerequisite; this is not a sandbox for an arbitrary interpreter. ## Author readiness Human owner: @gensokyo-dev Declarations entered under the user's explicit delegation; independent human review was not observed in this session. - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:46:34 +00:00