Repository navigation
PR D: CPU, device and compute evidence infrastructure - #20
Merged
Merged
Conversation
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b34b7b8a42
ℹ️ 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".
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
PR D adds executable CPU/device evidence infrastructure after PR A #17, PR B #18 and PR C #19. Runtime installation, API/device discovery, access, execution, correctness, actual backend and fallback are separate facts. The hosted CPU lane computes a fixed reference result and validates its evidence using the existing v1 Profile/Result model.
Base:
74801fe0e78bef71182612408d6f8e4a95f69f56Head:
0dd38027970de1345879bc5d740ec127f4a9b044Tested merge:
2f2c161fbd67ccae2ad5bbbe2fb4d6c2271795bdScope comes from the owner's PR D instructions and subsequent Codex/2026-10-17 independent Claude review follow-ups. Keep this PR open and unmerged.
Changes
compute.probe_contract: device-v1and typed probes extend schema version 1 without replacing the schema engine. Runtime/process, log/artifact, full run identity and stage requested/effective backend evidence must agree.[[4,10],[8,-1]]; deliberate incorrect-result/execution-error inputs fail. CPU capabilities inferred from the running interpreter are labelled inferred. Install/initialize describe this interpreter/reference starting, not package installation or browser preparation.device-tests.yml: one Ubuntu 24.04 hosted CPU job with an optional browser probe, strict missing-package/binary handling after setup, full paired Chromium without the unused shell download, a three-minute browser-step limit, strict validation, negative regressions and artifacts. CPU/evidence steps retain their existing failure conditions.Existing
ci.yml,compatibility.yml,python-abi.yml, 12 dev + 12 production E2E cases, product code and lockfiles are unchanged. No SDK/inference framework, deployment manifest, cluster/service setup, new Windows job, hardware/self-hosted runner, repository setting or privilege exception.Review follow-ups
The four earlier Codex P2 findings were addressed in
ed60c63. The owner's independent Claude review of74801fe..ed60c63identified no blockers/high findings and additional consistency/testing/documentation work.0dd3802addresses F1–F17 and the related improvements below.A forced GitHub Actions step timeout was not deliberately run. The code-level stalled-process tests execute in the regression suite, actual local Chromium SIGSTOP was tested, and the workflow has the additional step timeout. Those facts do not claim a separately observed Actions cancellation experiment.
Independent verification
GPT-6.1 Sol agents authored the changes. Two separate GPT-6 Astra agents independently reviewed code/security and performed adversarial rebuttal without repository edits. Both reported no remaining blocking or scoped-medium findings on the final diff. Review is evidence, not approval.
Independent runs from the repository root:
python -I scripts/ci/tests/test_compatibility_schema.pypython -I scripts/ci/tests/test_environment_probe.pypython -I scripts/ci/tests/test_python_abi_probe.pypython -I scripts/ci/tests/test_device_contract.pypython -I scripts/ci/tests/test_compute_probe.pypython -I scripts/ci/tests/test_device_environments.pynode --test frontend/scripts/probe-browser-compute.test.mjsgit diff --checkThe six Python suites comprise 327 tests, including the two stated skips. Author scratch checks killed 16 contract, 16 importer, 16 browser and 5 fixture mutants. Independent Astra checks reran all 16 contract and 16 browser mutants and constructed 8 importer guard mutants; all were killed. A separate measured→known provenance mutation failed both the same-value fixture and actual-Node CLI test.
Actual supplemental system Chromium with sandbox enabled passed normally. Independently stopping its owned process during evaluation and close produced failed reports in about 2.5s/1.7s with shortened test budgets, removed the owned PID and allowed Node to exit. No root, installation or sandbox bypass was used. This is system-override evidence, not paired hosted-browser qualification.
Author self-assessment additionally validated all 25 real profiles (0 errors/warnings) and the Device YAML, runner, timeouts, permissions and action pins. Local missing isolation evidence remains a policy limitation; no local policy bypass is represented as compliance.
Secret-heuristic correction and current results
The exact Python script from SECURITY-REVIEW step 7 was independently extracted unchanged and run with
python3 -I <step7.py> <base> <head>, plus each commit returned bygit rev-list --no-merges <base>..<head>.74801fe..ed60c6374801fe..0dd38028be7f31^..8be7f31b34b7b8^..b34b7b8ed60c63^..ed60c630dd3802^..0dd3802All leads were inspected as documentation, test names or test sentinels; no credential was identified. Exit 1 means leads required review, not a clean keyword scan. The earlier unqualified exit-0 statement did not describe the full PR range and is corrected here. Clean uncommitted mode returns exit 0 and is not evidence about committed history. This heuristic is not a dedicated secret scanner.
Latest-head Actions
All four workflows completed successfully on attempt 1: 31/31 jobs. These runs test head
0dd3802through merge2f2c161against base74801fe.The Device artifact was downloaded and independently revalidated by GPT-6 Astra against the final checkout: 0 errors, 0 warnings with the default policy and device profile. CPU requested/effective backend is
cpu; expected and actual matrix are[[4,10],[8,-1]]on CPython 3.12.15. All 168 Device regressions (58 + 45 + 10 Python, 55 Node) passed without skips.Device artifact 12088369791 contains ten files; downloaded ZIP SHA-256:
bd1420de3876ca9a8306c6b4b7592ba00b4370db5578251f3c4deb12aae2a7ea. Browser metadata recordsbinary_source: paired; all four browser probes are not-run (chromium-sandbox-unavailable). The environment digest and run identity match. No hosted Chromium WASM execution or accelerator kernel is claimed. All fifteen fixture pairs remain synthetic.The other Astra reviewer independently read all thirty jobs in the unchanged workflows. CI retained 12 dev + 12 production E2E passes, Django 23, pytest 23, Vitest 13, lock checks 49, and successful lint/type-check/build. Compatibility and ABI retain their existing explicit missing-wheel/lock/dependency blocks and skipped downstream checks. Installable ABI import/call checks passed; thread safety remains unverified. The seven existing Windows jobs retain their scoped administrator exception and
policy-skippedwarnings. Successful workflow conclusions do not promote these blocked lanes to supported.Historical hosted Device runs 38239325381 and 38242667040 passed CPU validation while every browser probe was not-run (
chromium-sandbox-unavailable). Neither executed browser WASM; Node unit tests exercised WASM separately. The latest artifact confirms the same hosted sandbox limitation.Security and remaining limits
Rootless First remains: application non-root execution, minimal device access, no rootful/privileged fallback, contents-read token, no persisted checkout credentials, full SHA action pins, no secrets/OIDC or self-hosted selection. Validation applies the default policy plus the profile's stricter engine-rootless requirement.
Chromium retains its sandbox, PATH/LANG-only environment, service-worker blocking and fixed intercepted origin. Its temporary loopback connection is not exposed in reports. No device identifiers, hostnames, environment dumps or raw Chromium errors are published by the producers.
Only environment/Result documents receive existing secret-shape validation. Raw browser/compute reports, CPU/test logs and the separate synthetic bundle rely on reviewed producers; there is no artifact-wide secret scan. Hash/run checks establish consistency, not authenticity against a dishonest producer.
No hardware lane is promoted to Checked. The CPU profile is not-assessed; security/performance remain not-run and overall compatibility incomplete. CPU reference evidence does not establish LLM inference or accelerator performance. No GPU/NPU kernel or real WSL2/cluster/systemd verification ran.
The existing PR C Windows administrator exception remains confined to its jobs. Windows non-administrator/token verification remains a separate follow-up with the 2026-11-08 review deadline. Future hardware testing needs actual hardware, an approved driver/runtime and correctness kernel, minimum device permissions and an isolated trusted runner at an owner-reviewed immutable revision; public PR code must never automatically select it.
Merge recommendation: ready within the defined PR D scope, based on the successful latest-head workflows and independent code/security/rebuttal reviews, with the explicit limitations and F24 follow-up above. This is evidence for the owner's decision, not automatic approval. The PR remains open and unmerged; no merge was performed.