Operational repos run no CI on a pull request into develop, contradicting D1.2

[ptr727]

Summary

On an operational repo, a pull request into develop runs no validation at all. The prescribed trigger set for the operational lint/validation workflow is push to develop plus pull_request to main plus workflow_dispatch (WORKFLOW.md:288), which has no pull_request trigger for develop. A PR opened against develop therefore matches nothing, the Validate sources job never starts, Check pull request workflow status job never reports, and the PR shows a clean mergeable state with an empty check list.

Observed on HomeAutomation-Config PR #34 (a compose.yml change, merged into develop): the only two entries in the status rollup were Merge dependabot pull request job and Disable auto-merge on maintainer push job, both SKIPPED, with mergeStateStatus: CLEAN. The lint gate ran only after the squash landed, as the advisory push run.

The hub's own test-pull-request.yml:5 already uses branches: [ main, develop ]. The gap is specific to the operational profile.

This contradicts D1.2 in the same document

WORKFLOW.md:145 states the behavioral contract as a MUST:

D1.2 A validation job always runs. Input: any PR. Output: a type-appropriate validation job runs unconditionally and the aggregator needs: it. […] Prevents: a PR merging with no validation […]

Section 6's operational trigger set makes the exact failure D1.2 names reachable, so the two halves of WORKFLOW.md disagree. Under "Operational is binary" (WORKFLOW.md:21) D1.2 is applicable to an operational repo (it has PRs and it has a validation job), so this reads as a defect rather than an N/A.

Why the carve-out is the wrong shape

Direct commit to develop is an allowance, not a substitute for review. GOVERNANCE.md:103 says the develop ruleset drops the PR and status-check gate, so direct signed pushes are allowed. Dropping a requirement does not remove the capability: a PR into develop stays perfectly legal, and on a change worth reviewing it is the better path. The model gates one of the two permitted paths and leaves the other open.

The asymmetry runs the wrong way. On the direct-commit path, CI is necessarily post-hoc, and advisory-after-the-fact is the accepted cost of the model. On the PR path the commit has not landed yet, so validation is both pre-merge and actionable, which is the one moment the gate is worth the most. That is where it currently does not run.

The promotion gate is real but late. A defect committed or merged into develop sits on the live surface until someone opens a develop -> main PR, which GOVERNANCE.md:103 describes as occasional. develop is the branch these repos actually deploy from.

Proposed change

Give the operational PR workflow the same pull_request scope the hub already ships, and keep the push trigger:

on:
  push:
    branches: [ develop ]        # advisory, covers the direct-commit path
  pull_request:
    branches: [ main, develop ]  # enforced, covers every PR
  workflow_dispatch:

Files that state the current shape and would move together:

• WORKFLOW.md:288 — the section 6 operational trigger prescription, the normative source.
• WORKFLOW.md:56-62 — the section 3 branch-model prose and diagram, which show develop reached only by direct signed commit -->|advisory CI| and omit the PR path entirely.
• GOVERNANCE.md:103-104 — "Operational Repositories", same omission.
• spec/project-types.json:146 — branch.operational.lintci describes the lint CI purely in terms of gating the promotion PR. If the trigger set changes, this assert is what an audit measures against.

Affected repos, per registry/repos.json: HomeAutomation-Config, ESPHome-Config, HomeAssistant-Config, Vantage-Config.

Tradeoff worth naming

Adding pull_request: develop means a PR-merged change is validated twice: once on the PR, once again by the push run when the squash lands. The concurrency group is keyed on ${{ github.workflow }}-${{ github.ref }}, so these are different refs and neither cancels the other. On a lint-only gate the duplicate is a couple of runner-minutes, and paying it looks better than the alternatives (a conditional that suppresses the push run has to distinguish a merge from a direct commit, which reintroduces exactly the reasoning this change removes). Worth stating in the docs rather than leaving a reader to discover the second run.

Questions

1. Should the develop ruleset also require the check on PRs, or only run it? These are separable. Running it means the PR shows red and the merge button warns. Requiring it means adding a required status check to repo-config/operational/develop.json, and the concern is whether that ruleset entry can bind PRs without also binding direct pushes — if it binds pushes, it breaks the direct-commit allowance that defines the model. Running-but-not-requiring is the conservative first step and delivers most of the value.
2. Does branch.operational.lintci become a letter check? It is verdict: intent today. With a concrete trigger set to assert, the trigger half could be checked mechanically.
3. Is there a reason for the original omission that this misses? The prescription is specific enough to look deliberate rather than accidental, and if the intent was "operational repos do not use PRs on develop", then the fix might belong in governance (say so explicitly) rather than in the trigger set — though PR Remove unnecessary condition for merging codegen pull requests #34 is evidence that they do.

[ptr727]

Fixed and merged to develop as 14c4fe3 (#587). Closing by hand, since a closing keyword fires only on a merge to the default branch and every hub PR targets develop.

The diagnosis held on both halves. The trigger set did make D1.2's own named failure reachable, and the carve-out was the wrong shape for the reason the issue gave: dropping the pull-request requirement permits the direct push without withdrawing the pull request, so the model gated one of two legal paths and left the other open. The asymmetry argument is what decided the fix, since the direct-commit path can only get post-hoc CI while on the pull-request path the change has not landed yet.

What landed, prose and spec only, as the hub ships no operational workflow YAML of its own:

• WORKFLOW.md section 3 carries the pull-request path in the branch-model diagram, plus the allowance-not-substitute reasoning and why the check is reported rather than required.
• WORKFLOW.md section 6 widens the prescription to pull_request with branches: [ main, develop ], keeps the 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 find 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 this came from.
• spec/project-types.json keeps branch.operational.lintci as intent and adds branch.operational.prtriggers as a letter check.
• TODO.md records the downstream application, which this change does not perform.

Answers to the three questions.

1. Run, do not require. A ruleset required_status_checks rule binds the branch rather than the pull request, so requiring the check on develop would gate the direct push as well and dissolve the allowance that defines the model. repo-config/operational/develop.json is untouched. The docs now state that reasoning rather than leaving it as an unexplained default.
2. The check splits. Linter composition stays intent, where equivalence is judged rather than matched. The trigger set is concrete enough to assert statically, so it became branch.operational.prtriggers, verdict letter.
3. No evidence the omission was deliberate. The hub's own test-pull-request.yml has always named both branches, and PR Remove unnecessary condition for merging codegen pull requests #34 shows operational repos do open pull requests into develop, so it is treated as a gap rather than an intent to encode.

One correction the review earned, worth recording here. The first draft described the second run as firing "when the squash lands", which reads a release-model constraint onto a branch that does not carry it: the operational develop ruleset prescribes no merge method at all, unlike repo-config/develop.json, which does require squash. The same sentence named the concurrency key as the ref alone when section 2 prescribes the workflow name plus the ref. Both are fixed, and the text now states the mechanism the argument rests on, that a pull_request run's ref is refs/pull/<n>/merge where the push run's is refs/heads/develop.

Still open, and not fixed by this. The four operational repos each need the one-line trigger widening in their own workflow. That is cross-repo work, carried by the new TODO.md "Fleet Chores" entry, and it needs the maintainer to name the repos.