Skip to content

ci: bootstrap opt-in intentional CI development - #6218

Open
rwgk wants to merge 6 commits into
masterfrom
rwgk/intentional-ci-dev-bootstrap
Open

rwgk wants to merge 6 commits into
masterfrom
rwgk/intentional-ci-dev-bootstrap

Conversation

@rwgk

@rwgk rwgk commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Restacking a chain of PRs currently starts CI for every rewritten PR, even when the author only wants to update the stack. Introduce an opt-in ci:intentional label that suppresses the 29 automatic GitHub Actions test jobs on participating PRs, independently of draft/readiness state. Existing conditions still apply to other PRs and to scheduled, branch-push, release, and manual runs.

Add a small experimental entry point for /ci full, /ci quick, and /ci help. During the pilot, any PR author can request runs on their own open PRs carrying ci:intentional. The listener reads the live PR state, captures the head/base SHAs, and dispatches a fixed workflow on intentional-ci-experiment. It never checks out PR code.

This is a bootstrap, not the finished CI implementation: the dispatched worker is an explicit placeholder that runs no tests and publishes no PR CI result. After this merges, I will create the shared intentional-ci-experiment development branch from master and iterate on the worker there without requiring review of each experimental step. The listener and ordinary workflows on master can remain stable. I will submit the completed design and any proposed project-wide switch together in a final PR.

The pilot documentation explains setup, separate experimental result/concurrency/cache names, the initial-label race, and existing-stack rebasing. The opt-out covers Actions test jobs; AppVeyor, Read the Docs, and the preview-link/label-maintenance workflows are separate.

Validation

  • Full pre-commit run --all-files passed, including GitHub workflow schema validation.
  • actionlint passed for all seven changed/new workflows.
  • GitHub's @actions/expressions evaluator passed 6,496 condition cases and eight listener event/authorization cases, covering labeled/unlabeled PRs, draft/ready states, ready_for_review, python dev, and non-PR events.
  • An independent baseline audit confirmed that all existing conditions and workflow definitions are preserved apart from the opt-out.

The final head has no CI skip instruction, so this update starts normal PR CI. Live comment-to-dispatch behavior can only be exercised after the entry point exists on the default branch; it has not been tested on GitHub yet.

@rwgk
rwgk requested a review from henryiii as a code owner October 25, 2026 21:53
@henryiii

Copy link
Copy Markdown
Collaborator

Fable 5.1 review:

🤖 AI text below 🤖

.github/workflows/ci.yml
● 9 [correctness] Removing the ci:intentional label never re-triggers CI (types lack labeled/unlabeled), so a PR can go from labeled to merged with zero test runs; the doc tells authors when to add the label but not that removal requires a new push or close/reopen.
● 36 [simplification] The identical two-line label guard is copy-pasted into 26 jobs across 6 workflows; any job added later without it (or any typo in one copy) silently runs for labeled PRs and nothing catches it.

.github/workflows/intentional-ci.yml
● 53 [security] The listener lets any PR author trigger a workflow_dispatch of the intentional-ci-experiment branch; workflow_dispatch runs on an in-repo branch receive repository secrets and whatever token permissions that branch's YAML declares, so the 'read-only, no secrets' rule in intentional-ci.md is only a convention the listener cannot enforce.
● 21 [usability] Recognized-but-rejected commands exit silently with a green job: startsWith() in the expression is case-insensitive and prefix-only, while the JS regex is exact, and the author/label/state check just returns, so the user gets no feedback at all.
● 4 [efficiency] issue_comment: created with a job-level if creates a workflow run record for every comment on every issue and PR in pybind/pybind11, not only /ci comments.
● 70 [usability] A 404/422 from createWorkflowDispatch is reported only in the listener's Actions job (pull-requests: read means no PR comment/reaction), so the PR author who posted /ci full has no signal that the request failed or succeeded.

.github/workflows/format.yml
● 25 [design] Skipping the pre-commit Format job on labeled PRs removes the only GitHub-side lint gate while giving nothing back: pre-commit.ci already runs on every PR regardless of label, so the opt-out saves about a minute and the clang-tidy job is the only expensive part.

.github/intentional-ci.md
● 22 [style] 'Any PR author is welcome to join the pilot.' is mannered prose and 'retaining'/'retain' (lines 4-5, 17) are non-Simplified-Technical-English verbs, per the global CLAUDE.md convention.

The review of PR #6218 (master...HEAD, 6 commits, 8 files) found 8 issues, reported above. The two that matter most:

  • Label removal never re-triggers CI. A PR can be labeled, pushed with all jobs skipped, unlabeled, and merged with zero test runs. Either document "push or close/reopen after removing the label" or add unlabeled to the trigger types.
  • Any PR author can dispatch the experiment branch, and those runs have secrets. The "read-only, no secrets" rule is only a convention. Pin the worker job to permissions: {} and consider a ruleset on the branch.

The rest are feedback gaps in the /ci listener (silent exits, no PR-side signal on dispatch failure), the 26-copy label guard, the skipped-run noise from issue_comment, the pre-commit opt-out being pointless since pre-commit.ci still runs, and some wording in intentional-ci.md.

@henryiii

Copy link
Copy Markdown
Collaborator

Also, I'd be fine to replace our draft/non-draft mechanism with this, even as an experiment.

This branch has not been deployed

No deployments
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