Skip to content

Verify tagged OpenAI ZIPs in Codex - #217

Merged
n3wth merged 15 commits into
google:mainfrom
Aaravkhanal:codex/verify-v0.1.0-release
Oct 30, 2026
Merged

n3wth merged 15 commits into
google:mainfrom
Aaravkhanal:codex/verify-v0.1.0-release

Conversation

@Aaravkhanal

@Aaravkhanal Aaravkhanal commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Contribution Type

  • Bug Fix / Minor Patch

Summary of Key Changes

  • Install the exact published OpenAI v0.1.0 ZIP through a temporary marketplace and compare every installed archive entry byte-for-byte.
  • Preserve per-skill SKILL.md read checks while mapping release ZIP underscore directories to current hyphenated skill names. Use checked-in marketplace metadata for checkout and ZIP installs.
  • Verify exact-artifact installation and fix the checksum command for downloading only the OpenAI ZIP. This is a focused contribution to Verify exact release bundles across hosts #60; other supported-host and release-note validation remain open. For Check TESTING.md and update test results #74, this PR records Codex release evidence; the setup-instructions fix landed in merged PR docs(testing): check Python version before creating the venv #183.
  • Sync the generated Claude distribution README with its source. This fixes the existing packaging consistency test that also failed on upstream main and was blocking the required unit workflow.

Human Accountability Affirmation

  • I am a human contributor. I have personally reviewed, understood, and can explain every line of this change in review (using AI tools to write code is welcome, but human understanding of the submitted change is required).

Verification

The v0.1.0 OpenAI ZIP was rebuilt from source commit b3a3595; its SHA-256 is 5f17b3942eaccc657621dfa550ef9fcaf626ad7d170085a9ef98a0e9e8afcefb, matching the release record. It passed its CRC check, contained 53 files, and every installed file matched the ZIP byte-for-byte. The command shasum -a 256 -c --ignore-missing SHA256SUMS was verified with only the OpenAI ZIP present.

Check Result Evidence
Exact artifact installation Pass All 53 files matched.
Fresh-context isolation Pass Native child returned UNKNOWN for the parent-only marker; marker absent from child trace.
Brainstorm, draft, review artifacts Pass Brief, draft, final design, and five stage reports produced; 15 native reviewers completed; final design matched draft; no implementation code written.
Installed skill reads Pass Trace confirmed reads of all three installed SKILL.md files. Release ZIP underscore directories map to their skill names.
Codex integration guide Pass The guide-read assertion lives only in test_brainstorm_draft_review; the isolation test no longer requests or asserts a guide read. The earlier live trace at commit 6e8ddda contained pwd; rg --files ...; cat of the installed egm_brainstorm/SKILL.md; its aggregated output included the full skill contents. The verifier initially missed it because it rejected the compound shell command. The final verifier handles literal semicolon-separated cat segments.

The full opt-in live suite was freshly rerun on code commit 8bd11c5 and both tests passed in 482.30 seconds on Codex CLI 0.164.0-alpha.11.1, model gpt-6.1-sol, medium reasoning, macOS. This run confirmed the path-free workflow, the installed guide and all three skills, all requested artifacts, and 15 native reviewers. Seven offline regression cases cover successful and failed reads, literal compound cat commands, and conservative rejection of && and pipe commands. The follow-up report-only commit is 7b42bbc.

The initial CI failure was test_published_claude_runtime_is_current: distributions/claude/README.md lagged the source by one line. Commit 889cde4 syncs the generated README; the full unit suite now passes locally with 145 passed on Python 3.13.3. The exact packaged Claude folder also matches fresh pack_plugin.py output, and git diff --check passes. The GitHub Python 3.12 unit matrix has not run on this commit because the workflow is waiting for maintainer authorization.

Other hosts were not tested. The live tests are opt-in and consume authenticated Codex model usage.

@google-cla

google-cla Bot commented Oct 29, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 9289fa0. The release-ZIP path is useful: extracting the tagged archive, installing it through a throwaway marketplace and byte-comparing every entry against the installed copy is the right shape, and the report is careful not to claim an end-to-end pass. pack_plugin.py writes only file entries, so iterating namelist() will not trip on directory entries.

[P2] The change to test_brainstorm_draft_review is not limited to release mode. It drops the per-skill SKILL.md read check for every run, including the checkout path the existing 0.163.0 row was recorded against, and replaces it with a directory-existence check that installation already guarantees. Details inline.

Smaller items: the report location conflicts with TESTING.md's guidance, and the release marketplace is hand-written rather than taken from the checked-in file. Both inline.

The test file compiles; I did not run the live Codex suite or download the release. Merge is also held on non-code items: the Human Accountability Affirmation is unchecked, so the scope/template check fails, cla/google fails, and the Tests workflow has not run on this head.

Comment thread tests/integration/test_codex.py Resolved
Comment thread tests/integration/test_codex.py Resolved

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at a57711a. The new commit gets the required-read check passing by giving Codex the installed docs/codex.md path in both prompts and telling it the read is required. The report and TESTING.md row now say Pass. My earlier P2 on the dropped skill-read check is still open, and the prompt change raises a new one. Details inline.

The PR description is now out of date. It still says the integration test failed and calls this a partial run, while the report and the new row say both live tests passed. Please update it to match.

The two P3s from last round are unchanged: the report is still under tests/reports/, and the release marketplace is still hand-written. The OpenAI ZIP does not include .agents/plugins/marketplace.json, so reading it from repo_root as suggested is still the simplest way to keep both paths on the same policy and category.

I did not run the live Codex suite. Merge is still held on non-code items: the Human Accountability Affirmation is unchecked, so the scope/template check fails, cla/google fails, and the Tests workflow has not run on this head.

Comment thread tests/integration/test_codex.py Resolved
Comment thread tests/integration/test_codex.py Resolved
Comment thread TESTING.md Resolved

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at d8a7910. This commit closes three of the four open items: the per-skill SKILL.md read check is back, resolved per directory so it matches the v0.1.0 underscore layout; the release marketplace now comes from the checked-in file; and the report is out of tests/reports/ with the row pointing at this PR.

[P2] Still open from last round: both prompts still give Codex the installed docs/codex.md path and tell it the read is required. The required-read assertion therefore measures whether Codex follows the prompt, not whether the plugin leads it to its guide, and the change applies to checkout mode as well. I'd still rather keep the path-free prompt and record the release run as partial. If you keep it, the row needs to say so; updated suggestion inline.

The PR description still says the integration test failed and calls this a partial run, while the row says Pass and now sends readers to this PR for the full run report. Please replace the Verification notes with the full report from the removed tests/reports/v0.1.0-codex-validation.md, updated for this run. Also confirm the 378.40s run was at d8a7910; the skill-file read assertion the row now cites only exists from this commit.

Smaller items inline: the documented checksum command fails as written, plus two optional simplifications.

The test file compiles; I did not run the live Codex suite. I ran the checksum command against the v0.1.0 release assets. Merge is still held on non-code items: the Human Accountability Affirmation is unchecked, so the scope/template check fails, and cla/google fails.

Comment thread TESTING.md Resolved
Comment thread TESTING.md Resolved
Comment thread tests/integration/test_codex.py Resolved
Comment thread tests/integration/test_codex.py Resolved
n3wth
n3wth previously approved these changes Oct 29, 2026

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 8e1e5f7. All open items from last round are addressed: both prompts are path-free again, the TESTING.md row records the release workflow as Partial with the guide-read failure and keeps the path-supplied run as a separate diagnostic, the checksum command uses --ignore-missing, and the marketplace and skill-directory simplifications are in. The PR description now matches the row.

The guide-read gap is now a known failing assertion in release mode, which is the honest outcome. Two small follow-ups inline, neither blocking. Please also note in the description which commit the 392.28s path-free run was on. The path-free prompt and the skill-read assertion only coexist from dcd73ac.

The test file compiles. I did not run the live Codex suite. Merge is still held on cla/google, which fails on this head.

Comment thread tests/integration/test_codex.py Resolved
Comment thread TESTING.md Resolved

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at fd4885d. The new commit changes successful_reads so a compound cat that exits non-zero still counts for each file whose opening 200 characters appear in the captured output. That is a reasonable fix for the false negative the description reports, and it stays conservative. I ran the helper against synthetic traces: a cat guide /missing that fails after printing the guide counts the guide; a failed cat with no matching output counts nothing; a failed read-typed command still counts nothing.

[P2] The PR description and the TESTING.md row now disagree on the main result. The description says the path-free workflow passed, including the guide read. The row still says Partial and that docs/codex.md was not read in the 392.28s run. Details inline.

Smaller items inline: the new trace-shape fallbacks don't appear to match what this function is given, and the verifier has no offline test. The required_reads round-trip nit from last round is still open; optional.

At fd4885d, 137 of 138 offline unit tests pass locally. The one failure is test_published_claude_runtime_is_current, which fails the same way on main and is addressed by #215. I did not run the live Codex suite. cla/google and the scope/template check now pass.

Comment thread TESTING.md Resolved
Comment thread tests/integration/test_codex.py Resolved
Comment thread tests/integration/test_codex.py Resolved

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 9d3cfbd. The row and description now agree, the prompts are path-free again, and the required_reads round-trip is gone. Thanks.

[P2] The row now says Pass, but it doesn't say how that result was produced. At fd4885d it said Partial for the 392.28s path-free run; the only change since is the row text, so I can't tell whether this is a fresh live run or that trace re-scored with the new verifier. Details inline.

[P3] The isolation test also drops its guide-read assertion, which isn't in the description. Inline.

Still open from last round, both P3: the trace-shape fallbacks in successful_reads (suggestion inline), and no offline test for the new verifier behaviour.

The test file compiles. I did not run the live Codex suite. cla/google and the scope/template check pass; the Tests workflow has not run on this head.

Comment thread TESTING.md Resolved
Comment thread tests/integration/test_codex.py Resolved
Comment thread tests/integration/test_codex.py Resolved
Comment thread tests/integration/test_codex.py Resolved

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 6e8ddda. The trace-shape fallbacks are gone, so successful_reads now only handles the rollout shape read_trace returns, and tests/unit/test_codex_traces.py pins the cases I asked for: a failed compound cat that printed the file counts, one without the contents doesn't, and a failed read-typed command doesn't. Thanks.

[P2] Still open: the row says Pass but not how it was produced. This head changed test_codex.py after the 392.28s run (the fallbacks were removed), so I can't tell which commit the live result is from or whether the verifier that now decides it was exercised live. Details inline.

[P3] The isolation test dropping its guide-read assertion is still not in the description. Inline.

One nit inline.

At 6e8ddda, all offline unit tests pass locally except test_published_claude_runtime_is_current, which fails the same way on main and is addressed by #215. I did not run the live Codex suite.

Comment thread TESTING.md Resolved
Comment thread tests/integration/test_codex.py Resolved
Comment thread tests/integration/test_codex.py Resolved

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at d800faf. Splitting the unknown command on ; and checking each cat segment on its own is a reasonable fix, and the new pwd; rg --files ...; cat <path> case in test_codex_traces.py pins it. I checked the tokenizer by hand: &&, | and ;; stay inside a segment and get rejected, so only a plain cat <absolute paths> segment counts.

[P2] Still open from the last two rounds: the row says Pass but not how it was produced. This commit changes the verifier again after the live run, so the result the row reports was not produced by the code now deciding it. Inline.

[P3] The isolation test dropping its guide-read assertion is still not in the description (test_codex.py:237). A sentence there is enough.

Two small items inline, both optional. I did not run the unit tests or the live Codex suite. cla/google and the scope/template check pass; the Tests workflow has not run on this head.

Comment thread TESTING.md Resolved
Comment thread tests/integration/test_codex.py Resolved
Comment thread tests/integration/test_codex.py Resolved
Comment thread tests/unit/test_codex_traces.py Resolved

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 8bd11c5. Both nits from last round are in, the new && and | cases pin the rejection paths, and the row now gives the commit and duration of the live run. I ran tests/unit/test_codex_traces.py locally: 7 passed.

Nothing blocking. Three small items inline: a stale case count in the row, the duration to confirm, and the isolation-test sentence for the description. I did not run the live Codex suite. cla/google and the scope/template check pass; the Tests workflow has not run on this head.

Comment thread TESTING.md Resolved
Comment thread TESTING.md Resolved
Comment thread tests/integration/test_codex.py Resolved
@Aaravkhanal
Aaravkhanal requested a review from n3wth October 29, 2026 09:33
@Aaravkhanal

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. I addressed the requested items and updated the report/PR description. The fresh full live run passed both tests in 482.30s at code commit 8bd11c5; 7b42bbc only updates the report row. The earlier pwd; rg --files ...; cat egm_brainstorm/SKILL.md trace was from commit 6e8ddda, and its captured output contained the full SKILL.md contents. The guide-read check now lives only in test_brainstorm_draft_review; the isolation test no longer asks for it. I added offline cases for compound reads and the conservative &&/pipe rejection cases. All inline threads are resolved; I’ve requested another review.

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 7b42bbc. The only change since 8bd11c5 is the TESTING.md row, and it now matches the description: 482.30s, 8bd11c5 as the live-run commit, and seven offline cases. test_codex.py and test_codex_traces.py are unchanged from what I reviewed last round (7 passed), so the live run still covers the code that decides it.

The pwd; rg --files ...; cat <path> question is answered in the thread: it was the 6e8ddda trace, and the compound-read case in test_codex_traces.py pins that shape. The isolation-test sentence is in the description now too.

Nothing further from me. I did not rerun the unit tests or the live Codex suite on this head. cla/google and the scope/template check pass; the Tests workflow has not run on this head.

@Aaravkhanal
Aaravkhanal requested a review from n3wth October 29, 2026 09:53
n3wth
n3wth previously approved these changes Oct 29, 2026
n3wth
n3wth previously approved these changes Oct 29, 2026

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 889cde4. The only change since 7b42bbc is the one-line distributions/claude/README.md sync, and it makes the packaged README match the root README.md on current main f0ba151 byte for byte. That fixes test_published_claude_runtime_is_current. test_codex.py, test_codex_traces.py and the TESTING.md row are unchanged from what I last reviewed, so the live run still covers the code that decides it.

At 889cde4, all 145 offline unit tests pass locally. No remaining finding in this change. I did not run the live Codex suite. cla/google and the scope/template check pass; the Tests workflow has not run on this head and needs maintainer authorization, so merge must wait for it.

Comment thread distributions/claude/README.md Resolved
Comment thread tests/unit/test_codex_traces.py Resolved

@n3wth n3wth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the release-ZIP integration fixture and conservative trace parser, including the final Windows-only fixture corrections. All 145 offline tests and current Linux/Windows/security checks pass. The native v0.1.0 ZIP run remains documented as dated evidence, without claiming a new live run.

@n3wth
n3wth merged commit 403d888 into google:main Oct 30, 2026
10 checks passed
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