Skip to content

Run CI on a pull request into an operational develop - #587

Merged
ptr727 merged 2 commits into
developfrom
feature/585-operational-pr-into-develop
Aug 6, 2026
Merged

Run CI on a pull request into an operational develop#587
ptr727 merged 2 commits into
developfrom
feature/585-operational-pr-into-develop

Conversation

@ptr727

@ptr727 ptr727 commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Closes #585 (a merge into develop fires no closing keyword, so this is closed by hand after promotion).

What was wrong

The prescribed trigger set for the operational lint/validation workflow was push to develop plus pull_request to main plus workflow_dispatch. A pull request opened against develop therefore matched nothing: the validation job never started, the aggregator never reported, and the pull request showed a CLEAN mergeable state with an empty check list. WORKFLOW.md D1.2 states as a MUST that a validation job runs on any pull request, so section 6's trigger set made D1.2's own named failure reachable and the two halves of the document disagreed.

The framing the issue got right, and the one this change is built on: direct commit to develop is an allowance, not a substitute for review. The ruleset drops the pull-request requirement, which permits a direct push without withdrawing the pull request. On the direct-commit path CI can only be advisory after the fact, and that is the accepted cost. On the pull-request path the change has not landed yet, so validation is pre-merge and actionable, which is the one moment it is worth the most, and that is where it was not running.

What changed

  • WORKFLOW.md section 3 carries the pull-request path in the branch-model diagram and a paragraph on why the allowance does not remove the pull request, why the asymmetry runs the wrong way, and why the check is reported rather than required.
  • WORKFLOW.md section 6 widens the operational trigger prescription to pull_request with branches: [ main, develop ], keeps push to develop, states that naming main alone is a defect and what it produces, and names the duplicate-run tradeoff outright rather than leaving a reader to discover the second run.
  • GOVERNANCE.md "Operational Repositories" gains the matching bullet, since the omission was there too.
  • repo-config/README.md reads the dropped ruleset rules as an allowance rather than a prohibition, which is the misreading the gap came from.
  • spec/project-types.json keeps branch.operational.lintci as intent and adds branch.operational.prtriggers as a letter check on the trigger set.
  • TODO.md records the downstream application across the four operational repos, which this change does not perform.

Answers to the issue's three questions

  1. Should the develop ruleset also require the check, or only run it? Only run it. A ruleset required_status_checks rule binds the branch, not the pull request, so requiring the check on develop would gate the direct push as well and dissolve the allowance that defines the model. Reported-but-not-required is what shipped, and the docs now say why rather than leaving it as a default.
  2. Does branch.operational.lintci become a letter check? It splits. The linter composition stays intent, since equivalence there is judged rather than matched. The trigger set is now concrete enough to assert statically, so it becomes its own letter check, branch.operational.prtriggers.
  3. Was the omission deliberate? Nothing found in the tree or history argues it was. The hub's own test-pull-request.yml has always named both branches, and HomeAutomation-Config PR Remove unnecessary condition for merging codegen pull requests #34 is evidence that operational repos do use pull requests into develop, so this is treated as a gap rather than an intent to encode.

Scope

Hub prose and spec only. The four operational repos (HomeAutomation-Config, ESPHome-Config, HomeAssistant-Config, Vantage-Config) each still need the one-line trigger widening in their own workflow, which is cross-repo work carried by the TODO.md entry and needs the maintainer to name the repos.

Verification

All local gates green on this branch: test_prose_lint.py (163), test_repo_gate.py (23), test_pr_review.py (82), spec/audit.py --selftest, repo_gate.py, spec/validate.py, jq empty over every JSON manifest, editorconfig-checker, markdownlint-cli2 over 44 files (0 issues), and prose_lint.py --diff origin/develop clean on both the gating and warn-only rule sets. CRLF preserved on all four Markdown files and the JSON.

🤖 Generated with Claude Code

The operational lint workflow's prescribed trigger set was push to develop
plus pull_request to main, so a pull request opened against develop matched
no trigger at all: no validation job, no aggregator report, and a CLEAN
mergeable state with an empty check list. That is the exact output D1.2
forbids on any pull request, so the two halves of WORKFLOW.md disagreed.

Direct commit to develop is an allowance rather than a substitute for
review. The ruleset drops the pull-request requirement, it does not
withdraw the pull request, and on the pull-request path the change has not
landed yet, which is where validation is worth the most.

Widen the prescription to pull_request: branches: [ main, develop ], keeping
the push trigger, and record the duplicate run a merged pull request now
costs. The result is reported and not required on develop, since a required
status check there would gate the direct push too.

Add branch.operational.prtriggers as a letter check, since the trigger set
is now concrete enough to assert, and note the four downstream operational
repos that still need the one-line change.

Refs #585

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 6, 2026 14:04

Copilot AI 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.

Pull request overview

Aligns the operational workflow model documentation and spec with the contract that validation must run for any pull request, including PRs targeting develop, and adds a new spec check to statically assert the required trigger set.

Changes:

  • Update WORKFLOW.md and GOVERNANCE.md to explicitly describe PRs into develop for operational repos and require pull_request triggers for both main and develop.
  • Update repo-config/README.md to clarify that the relaxed develop ruleset is an allowance (PRs remain valid) and that CI should run on PRs into develop.
  • Extend spec/project-types.json with a new letter check (branch.operational.prtriggers) and add a TODO item to apply the downstream workflow change in the four operational repos.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
WORKFLOW.md Documents the operational PR path into develop and prescribes pull_request triggers for both branches.
GOVERNANCE.md Adds an operational-model bullet clarifying PRs into develop remain valid and must run CI.
repo-config/README.md Clarifies the operational develop ruleset’s intent and how CI behavior relates to PRs into develop.
spec/project-types.json Adds a new letter check to assert the operational lint workflow trigger set includes PRs into develop.
TODO.md Records the required downstream per-repo workflow change for the operational repos.

Comment thread WORKFLOW.md Outdated
The operational develop ruleset prescribes no merge method, so describing
the second run as the one that fires "when the squash lands" reads a
release-model constraint onto a branch that does not carry it.

The concurrency claim was also loose. The prescribed group is the workflow
name plus the ref, not the ref alone, and what makes the two runs
independent is that a pull_request run's ref is refs/pull/<n>/merge while
the push run's is refs/heads/develop. Naming both refs states the mechanism
rather than asserting the outcome.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 6, 2026 14:11

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 14c4fe3 into develop Aug 6, 2026
7 checks passed
@ptr727
ptr727 deleted the feature/585-operational-pr-into-develop branch August 6, 2026 14:19
ptr727 added a commit that referenced this pull request Aug 6, 2026
…, and the operational pull-request trigger (#589)

Promotes the five commits develop carried since #582: #584 (a Copilot refusal is not coverage), #586 (the reference-definition collation), #587 (CI on a pull request into an operational develop), #588 (hub-hosted tooling, de-vendoring configure.sh), and #590 (the two definition blocks #588 pushed out of order).

Merge commit, no squash, develop not deleted. git merge-tree reported no conflict and wrote a tree byte-identical to origin/develop.

GOVERNANCE.md is verbatim-carried and three of these commits change it, so a fleet re-vendor is owed, including an entirely new carried section. #588 also turns every downstream repo-config/configure.sh copy into a deletion rather than a convergence, and #587 leaves a one-line trigger change owed by four operational repos.
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