Skip to content

TEMP: prove CI catches an HPOS violation - #5

Closed
YonatanVol wants to merge 2 commits into
mainfrom
test/ci-negative-check
Closed

YonatanVol wants to merge 2 commits into
mainfrom
test/ci-negative-check

Conversation

@YonatanVol

Copy link
Copy Markdown
Owner

Temporary. Verifies the negative half of Issue #4's acceptance criteria: CI must go red on a violation. Closed and deleted as soon as the run reports.

YonatanVol and others added 2 commits August 5, 2026 19:06
Runs the same gates CI will enforce on every pull request, so "it passed
locally" stops being the only evidence.

Jobs:
- php: PHPCS, PHPStan, HPOS sniff self-test and PHPUnit, on a 8.2/8.3 matrix
- secrets: gitleaks over full history
- verify: aggregates the above into one check

Matrix rationale. 8.2 is the supported floor and what Composer resolves
against; 8.3 is what local development and the production host run. A break on
either is a real break, so fail-fast is off — knowing both results is worth
more than finishing thirty seconds sooner.

The verify job exists so branch protection has a single stable check to
require. Requiring matrix legs by name means protection silently stops covering
any leg added later, which is the kind of gap nobody notices until it matters.

gitleaks checks out full history rather than the tip. On a public repository a
secret removed in a later commit is still public, so scanning only the tip
would miss exactly the case that matters.

Dependabot covers Composer and GitHub Actions weekly. The npm ecosystem is
deliberately absent: there is no JavaScript toolchain yet, because
registry.npmjs.org is unreachable from the development environment. Adding the
ecosystem before the manifest exists would produce noise, not updates.

Refs #4

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c1c4e02d-75c0-4597-8085-5e8b25d0d1bb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@YonatanVol

Copy link
Copy Markdown
Owner Author

Verified: CI failed on both PHP 8.2 and 8.3 with ElectricChic.HPOS.NoDirectOrderMeta.PostMeta, and the aggregate All checks passed job failed with them. Negative half of Issue #4's acceptance criteria confirmed. Closing — this branch was never intended to merge.

@YonatanVol YonatanVol closed this Aug 5, 2026
@YonatanVol
YonatanVol deleted the test/ci-negative-check branch August 5, 2026 16:09
YonatanVol added a commit that referenced this pull request Aug 6, 2026
Makes CI enforcing rather than advisory, and puts the rules themselves under
review.

Adds:
- Ruleset on main: pull request required, CI required, linear history, no force
  push, no deletion, no bypass actors
- Five issue-form templates driven by the Definition of Ready
- Pull request template requiring RTL and mobile evidence, and rollback notes
- CODEOWNERS over availability, pricing, integrations, returns, CI and the sniff
- The project label set; GitHub's stock labels removed
- Ruleset JSON in docs/operations/rulesets/ so protection is reviewable in a
  pull request instead of living only in a settings screen

Deviation from the plan, stated rather than hidden:

The plan requires review before merging. That is NOT implemented. This
repository has one collaborator, and GitHub does not permit approving your own
pull request, so requiring an approval would block every merge permanently.

The alternative — require an approval and add the owner as a bypass actor —
would look enforced while enforcing nothing, and would hide which merges skipped
review. A rule that must be routinely bypassed is worse than no rule, because it
teaches everyone that the rules are theatre.

So required approvals is 0, every other gate is real, no actor can bypass, and
the gap is documented as unmet rather than dressed up.
main-protection-with-review.json turns it on the moment a second maintainer has
commit access; CODEOWNERS is already written and becomes binding then.

The owner is deliberately not a bypass actor. Touching main directly requires
editing the ruleset first, which is a logged administrative action — unlike a
silent bypass.

Refs #5

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant