Skip to content

ci: block new workspace crate features#16455

Merged
bolinfest merged 1 commit intomainfrom
pr16455
Apr 1, 2026
Merged

ci: block new workspace crate features#16455
bolinfest merged 1 commit intomainfrom
pr16455

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 1, 2026

Why

We already enforce workspace metadata and lint inheritance for codex-rs manifests, but we still allow new crate features to slip into the workspace. That makes it too easy to add more Cargo-only feature permutations while we are trying to eliminate them.

What changed

  • extend verify_cargo_workspace_manifests.py to reject new [features] tables in workspace crates
  • reject new optional dependencies that create implicit crate features
  • reject new workspace-to-workspace features = [...] activations and default-features = false
  • add a narrow temporary allowlist for the existing feature-bearing manifests and internal feature activations
  • make the allowlist self-shrinking so a follow-up removal has to delete its corresponding exception

Stack created with Sapling. Best reviewed with ReviewStack.

Copy link
Copy Markdown
Contributor

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

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: c76e4380b4

ℹ️ 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 on lines +76 to +77
for path in cargo_manifests():
if errors := manifest_errors(
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.

P1 Badge Validate root workspace dependencies for feature toggles

The new policy checks are only applied to manifests returned by cargo_manifests() in this loop, which excludes codex-rs/Cargo.toml; that leaves a bypass where someone can add features = [...] or default-features = false for an internal crate under [workspace.dependencies] in the workspace root and still pass this script. Because Cargo propagates workspace dependency settings to crates using workspace = true, this means new internal feature activations can still be introduced without being caught.

Useful? React with 👍 / 👎.

## Why

We already enforce workspace metadata and lint inheritance for `codex-rs` manifests, but we still allow new crate features to slip into the workspace. That makes it too easy to add more Cargo-only feature permutations while we are trying to eliminate them.

## What changed

- extend `verify_cargo_workspace_manifests.py` to reject new `[features]` tables in workspace crates
- reject new optional dependencies that create implicit crate features
- reject new workspace-to-workspace `features = [...]` activations and `default-features = false`
- add a narrow temporary allowlist for the existing feature-bearing manifests and internal feature activations
- make the allowlist self-shrinking so a follow-up removal has to delete its corresponding exception
@bolinfest bolinfest merged commit dc263f5 into main Apr 1, 2026
36 of 44 checks passed
@bolinfest bolinfest deleted the pr16455 branch April 1, 2026 18:06
@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant