Skip to content

fix: route verification changes and require confirmed docs-only pushes - #4880

Merged
seratch merged 3 commits into
mainfrom
codex/maintenance-ci-change-routing
Sep 5, 2026
Merged

fix: route verification changes and require confirmed docs-only pushes#4880
seratch merged 3 commits into
mainfrom
codex/maintenance-ci-change-routing

Conversation

@seratch

@seratch seratch commented Sep 5, 2026

Copy link
Copy Markdown
Member

This pull request fixes skipped CI checks when verification scripts, workflow controls, or typecheck configuration change. Documentation deployment now uses the shared change detector, fetches missing event commits, and requires a successful, nonempty docs-only diff. Mixed changes, failed comparisons, and code-to-docs renames cannot authorize deployment.

@seratch seratch added the project label Sep 5, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 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-09-05T11:14:19.778280Z 27919cb Manual request
🔒 Security Review Completed 2026-09-05T11:14:54.595093Z 27919cb Manual request
ℹ️ 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.

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

I went through the failure semantics here as well as the expanded path routing. The asymmetry in unknown_changes() is the important part: uncertainty forces verification to run, while the same uncertainty cannot authorize a docs deployment. That is the safer default in both directions.

Fetching both endpoint commits before diffing also closes the shallow-checkout and force-push cases, and using --no-renames -z is a good detail: a code-to-docs rename still exposes the removed code path to the verifier, while unusual path names cannot corrupt the file list. The new tests exercise those cases directly, including failed diffs, missing SHAs, empty diffs and mixed pushes. I also checked the docs/scripts/ overlap: treating those files as both documentation and code inputs makes sense because generator changes need verification even when the resulting push remains docs-only.

This is a substantial improvement over having separate ad-hoc change detection in the workflow. I don’t see another routing gap in the current change set.

@seratch
seratch force-pushed the codex/maintenance-ci-change-routing branch from 83d33eb to bd13700 Compare September 5, 2026 10:34

@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: 96b96234c6

ℹ️ 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/scripts/detect-changes.sh Outdated
@seratch

seratch commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

@sylvesterkaczmarek Could you please refrain from posting review comments when you do not have any specific issues to raise? We would like to keep PR discussions focused on actionable feedback and direction from the maintainers.

@seratch
seratch enabled auto-merge (squash) September 5, 2026 11:16
@seratch
seratch merged commit 1d471a4 into main Sep 5, 2026
18 checks passed
@seratch
seratch deleted the codex/maintenance-ci-change-routing branch September 5, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants