Skip to content

fix(release): harden release workflow validation - #622

Open
mldangelo-oai wants to merge 9 commits into
mainfrom
mdangelo/codex/harden-release-integrity
Open

fix(release): harden release workflow validation#622
mldangelo-oai wants to merge 9 commits into
mainfrom
mdangelo/codex/harden-release-integrity

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Tighten release workflow validation so pull request titles, reviewed release notes, and successful CI runs are interpreted consistently across every active release path.

Changes

  • Centralize reviewed release-note validation, historical summary-marker parsing, and release-note composition in the existing release automation helper.
  • Route protected npm publication, release cutting, and GitHub release composition through the shared release-note trust boundary while keeping historical recovery exempt.
  • Preserve existing and generated GitHub release-note bytes through file-based composition, and normalize complete terminal CRLF/LF sequences before reconciliation.
  • Preserve existing lowercase Conventional Commit title types and reject embedded or trailing line breaks without losing title bytes in the label workflow.
  • Isolate body-only pull request edits from full CI concurrency and required context names while keeping title, base, code, and push events on the full matrix.
  • Require workflow-run release cuts to follow a successful push-triggered node-ci run on main, while retaining the explicit manual-dispatch path.
  • Document the existing lowercase title grammar and canonical leading marker block, with behavior-level regression coverage for custom title types, malformed, inline, duplicate, CRLF, Unicode-blank, and NUL-bearing inputs.
  • Resolve a usable Python interpreter in the existing Windows scan-history regression instead of selecting a nonfunctional launcher alias.

Testing

  • Failing-first release/workflow regressions: 218 passed, 32 failed, 519 expectations on the merged baseline.
  • Failing-first Unicode blank-summary regressions: 0 passed, 4 failed, 4 expectations before the protected publisher used the shared validator.
  • Failing-first NUL regressions: 0 passed, 5 failed, 5 expectations; the shell proof reproduced a dropped NUL and empty reviewed-summary marker block.
  • Failing-first historical-note NUL regression: 1 passed, 1 failed, 2 expectations; the repaired slice passed 2 tests with 0 failures and 4 expectations.
  • Failing-first macOS decoder regression: 0 passed, 1 failed, 1 expectation; the repaired table slice passed 4 tests with 0 failures and 20 expectations.
  • Failing-first body-edit concurrency regression: a hosted body-only edit cancelled 16 active Windows coverage jobs and failed both required Windows summaries; the local semantic gate failed 8 tests before the repair and passed all 8 with 60 expectations afterward.
  • Failing-first terminal-CRLF recovery regression: 0 passed, 1 failed, 1 expectation; the repaired case passed with 5 expectations.
  • Failing-first generated-note CRLF reconciliation regression: 0 passed, 1 failed, 3 expectations; the repaired case passed with 4 expectations.
  • Failing-first generated-note NUL transport regression: 0 passed, 1 failed, 3 expectations; the repaired CRLF and NUL table slice passed 2 tests with 0 failures and 8 expectations.
  • Failing-first title compatibility regressions: 10 passed, 6 failed before restoring existing custom title types across validation and release categorization.
  • Focused release automation, package skeleton, and Windows scan-history suites: 289 passed, 1 expected Windows-only skip on macOS, 0 failed, 734 expectations.
  • Python interpreter-resolution regressions: 3 passed, 2 expected Windows-only skips on macOS, 0 failed.
  • Bash 3.2.57 optional-argument workflow fragments: 2 passed, 0 failed, 6 expectations.
  • Formatting, TypeScript types, build, Node syntax, 10 YAML files, and git diff --check: passed.
  • Full TypeScript suite: 1,733 passed, 28 expected skips, 0 failed, 22,245 expectations across 92 files.
  • Package validation: packed @openai/codex-security@0.1.18; validated 265 archive entries, public import, NodeNext types, CLI, 113 bundled plugin files, bundled runtime, and nested-worker smoke behavior. The 1,224,544-byte tarball remained unchanged.

Risk and rollout

The change affects release gating, CI scheduling, and note composition, so an incorrect rule could block a release. Table-driven workflow tests cover publish and recovery modes, cross-platform title decoding and parity, marker, NUL, and terminal-line-ending boundaries, existing-release reconciliation, and direct-tag publication. No dependency or package payload changes are introduced.

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.

@github-actions github-actions Bot added the bug Something isn't working label Aug 24, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 24, 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
📝 Code Review Completed 2026-08-25T01:37:47.430178Z 6957d2d Manual request
🔒 Security Review Completed 2026-08-24T01:57:42.015809Z 1f5f3f7 PR opened
ℹ️ 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.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f5f3f767b

ℹ️ 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".

Comment thread .github/workflows/node-ci.yml Outdated
Comment thread sdk/typescript/scripts/release-automation.mjs
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 4dee051462

ℹ️ 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".

@kmbroai kmbroai 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.

Critical review

Reviewed head 4dee05146293d64ca608e03a6435a6e3616fc612.

Recommendation: keep the release-note and CI-event correctness fixes; separate the new title-type policy from them. No remaining execution defect was reproduced in the focused checks.

Necessary changes

File-based release-note transport avoids shell command substitution changing the bytes being validated. Sharing reviewed-summary parsing across release cutting and protected publication eliminates inconsistent interpretations. The current GitHub-release composition path normalizes complete terminal CRLF/LF sequences; the earlier dangling-CR recovery finding is addressed. Historical generated-only releases remain distinct from new releases that require reviewed notes.

The body-edit CI isolation also addresses a concrete workflow problem: a metadata edit must not cancel the code-validation run or replace its required context with a skipped success. The separate concurrency suffix, metadata-only context prefix, and required summaries work together; removing only one of them would undermine the fix. Release cutting now requires a successful push-origin run while preserving explicit dispatch.

Unnecessary coupling / behavior changes

The new 12-type allowlist is a policy restriction, not required to reject embedded newlines or classify release notes. The previous lowercase syntax accepted titles such as security: ... and deps: ..., and categorization already had an “Other changes” case. Updating the documentation makes the restriction explicit but does not establish why rejecting those titles is necessary. I would retain the generic lowercase type grammar and the newline checks, or make the closed set a separately approved policy change. This is a scope/compatibility recommendation, not a claim that the implementation disagrees with its new documentation.

Also call out that the replacement Unix required-summary jobs depend on the aggregate test matrix: a failure in another Unix/Node matrix entry now fails both node-22 summary contexts. That is broader than merely isolating body edits and should be intentional.

Verification

Ran release-automation.test.ts and skeleton.test.ts: 286 passed, 0 failed, including workflow-fragment and release-note regressions. Linux, Bun 1.3.14 / Node 22.13.1, cached dependencies. No hosted Actions run, branch-protection configuration change, release dispatch, tag, npm publish, or GitHub Release mutation was performed.

faizan-oai
faizan-oai previously approved these changes Aug 24, 2026

@faizan-oai faizan-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the workflow and release-note changes. The release automation and skeleton suites pass (286 tests), and required CI is green on this head. No blocking findings.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review the current head 0f64d08.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 0f64d08276

ℹ️ 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".

@faizan-oai faizan-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed 0f64d08276247176236e18c50af9190870c60f40: no blocking code finding, and all 289 focused release/workflow tests pass.

Approval is waiting on the failed Windows Node 22 check. The unchanged workbench-scan-root-alias.test.ts:48 receives status: null from its Python subprocess; the log omits the spawn error, so the cause is unproved. I am not treating this as a demonstrated regression from the title changes. Please get the required full CI passing; the later successful metadata-only run does not cover this failure.

@faizan-oai faizan-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed 6957d2d90699928a2a780c74485d3f5216b559dd against current main (ce452245de3d). The Windows test now uses the existing Python resolver; its path-alias and sibling-exclusion assertions are unchanged. No blocking findings.

Local verification: 296 focused tests passed, with 3 Windows-only skips; typecheck, generated-model check, changed SDK-file formatting, and diff checks passed. Full current-head CI passed all 27 jobs, including all three required Node 22 checks.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review (current head: 6957d2d)

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 6957d2d906

ℹ️ 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".

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants