Repository navigation
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. |
![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: 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
1 participant
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 completes the compute/device evidence pipeline after PR A #17, PR B #18 and PR C #19. The hosted CPU lane performs a real fixed matrix calculation and checks its result using the existing v1 Profile/Result model. Discovery, installation, availability, execution, correctness and actual backend remain separate facts.
Base:
74801fe0e78bef71182612408d6f8e4a95f69f56Head:
ed60c638aee8c237b950337df1df849f18301d9aTested merge:
379424f1490c3bde1e9fb4055c04dcba58978f55Requested scope: the owner's PR D implementation and subsequent Codex-review follow-up in this session. Keep this PR open and unmerged; no automatic merge.
Changes / Scope
compute.probe_contract: device-v1and typed backend probes distinguish runtime installation, API/device enumeration, access, completed operation, correctness, actual backend, software fallback, runtime/process and log/run evidence. Schema version and the existing schema engine remain unchanged.probe_compute.pyextends the collector's Result. Standard-library CPU multiplication returns[[4,10],[8,-1]]; deliberate mismatch/error cases fail. Required CPU stages record requested/effective CPU with matching operation evidence.device-tests.yml: one Ubuntu 24.04 hosted job, CPU/browser evidence, strict validator, negative regressions, summaries and artifacts. Six requested documents explain the interface and limits.Existing
ci.yml,compatibility.yml,python-abi.yml, the 12 dev + 12 production E2E cases and lockfiles remain unchanged. No SDK/inference framework, product API/DB/auth change, deployment manifest, real cluster/systemd setup, new Windows job, hardware/self-hosted runner, repository setting change or privilege exception.Codex review follow-up
The manual Codex code review of
b34b7b8reported four P2 findings. Independent Astra reproduction confirmed each before changes; commited60c63addresses all four.runtime_installed: true. False, unknown and merely declared installation cannot support those claims.GPT-6.1 Sol agents authored the code, tests and documentation changes. Separate GPT-6 Astra agents performed independent code/security verification and adversarial rebuttal without editing repository files. The original four invalid inputs are rejected; 35 additional independent negative/positive variations had no unexpected outcomes. No remaining scoped or blocking findings. Reviews are evidence, not approval or a human review.
The repository's automatic security review in the Codex summary applies to
8be7f31; the manual code review applies tob34b7b8. Neither is represented as an automated review ofed60c63.Validation
Author tests and independent Astra runs from the repository root:
python -I scripts/ci/tests/test_device_contract.py: 38 tests, OK.python -I scripts/ci/tests/test_compute_probe.py: 30 tests, OK, including real CLI denials and successful/failed operation outputs.python -I scripts/ci/tests/test_device_environments.py: 9 tests, OK.node --test frontend/scripts/probe-browser-compute.test.mjs: 5 passed.git diff --checkand the security review's added-line secret heuristic: exit 0.Independent actual local collect → browser → CPU/import runs used Python 3.12.14, Node 24.19.0 and UID 1000. CPU produced the expected matrix. The paired browser was missing and honestly not-run. Supplemental system Chromium 151.0.7922.173, with sandbox enabled, executed WASM 42; WebGL context existed, WebGPU API existed without an adapter and WebNN API was absent. Hardware computations stayed not-run. These local versions are not the committed hosted pins.
Local default policy correctly rejected two pre-existing container-isolation visibility uncertainties; explicit structural-only validation passed with policy-skipped/profile-unchecked warnings. This is not a local policy pass. CI contains no policy bypass.
Latest-head Actions: all four workflows succeeded, 31/31 jobs, attempt 1. The logs bind to the head/base/tested merge SHAs above:
Latest Device job 114788851557 recorded Python 3.12.15/Node 24.21.0, UID/real/saved UID 1001, superuser false, exact CPU matrix correctness, and default-policy/profile validation 0 errors, 0 warnings. Tests: 38 contract + 30 compute + 9 environment + 5 Node = 82 passed. Browser probes remain not-run: chromium-sandbox-unavailable; no unsandboxed retry. Security/performance remain not-run and overall incomplete.
An independent Astra reviewer downloaded artifact 11743167371, verified ZIP SHA-256
1ea50d07bc8829cc77508331ecf6b826034dd0b117348ca3e8bafeb82ce00019, matched environment/browser/CPU/run evidence and revalidated the downloaded Result with the final checkout and default policy: exit 0, 0 errors, 0 warnings. No new finding.Existing matrix jobs retain expected missing-wheel/dependency blocks and Windows policy-skipped limitations; successful workflow completion does not convert those into platform or non-administrator support. No retries were needed for the latest head.
Historical baseline
b34b7b8: all four workflows passed, including dev12/production12 E2E. Its downloaded Device artifact also passes the strengthened validator with the real profile/default policy: 0 errors, 0 warnings. That artifact recorded CPU success, UID1001, and all browser probes not-run due tochromium-sandbox-unavailable; it demonstrates compatibility with prior valid evidence, not hosted execution of the latest commit. Baseline macOS artifact upload needed one targeted retry after DNS ENOTFOUND; the retry succeeded.Implementation Status / Security
No hardware/support lane is promoted to Checked. The reference profile remains not-assessed; CPU pipeline correctness does not qualify LLM inference, accelerator performance or the application's dependency lock. Security/performance stages remain not-run, so overall compatibility is incomplete. No feature or SEC requirement status changes.
Read-only contents permission, credentials not persisted, full action SHA pins, no secrets/OIDC, no root/sudo/admin escalation, no privileged/rootful fallback and no self-hosted selection. Chromium sandbox stays enabled; inability to use it yields not-run without an unsandboxed retry. Raw driver output, host/account names and device serials are not published.
Independent security verification traced environment/browser input → consistency checks → typed evidence/stages → reports and diagnostics. It found no new command, network destination, credential handling or privilege mechanism. Local/artifact limitations are recorded above.
The PR C Windows administrator exception is unchanged and applies only to its existing two jobs. Risk acceptance for PR #19 does not establish non-administrator execution. Windows non-administrator/token probing remains follow-up with the 2026-11-08 review deadline.
Remaining limits
No GPU/NPU kernel, real WSL2/cluster/systemd service or hardware runner was executed. Future hardware verification requires actual hardware, a compatible approved runtime/driver, a reviewed correctness kernel, narrowly granted device access and an isolated trusted runner executing an owner-reviewed immutable revision. Public PR code must never automatically select it. Consistency checks and hashes do not attest a malicious producer.
Merge is recommended for the bounded PR D validation-infrastructure scope: the four Codex findings are addressed, independent Astra verification/rebuttal has no remaining blockers, and all latest-head Actions workflows pass. The PR remains open and unmerged; do not merge as part of this task.