Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/scripts/check-review-answered.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,10 +563,10 @@ def reply_age_seconds(stamp: str, now: str | None = None) -> float | None:
# a person who wants that merges by hand;
# 3. every thread the automatic reviewer opened has a reply from a person (`reviewer_threads`,
# the SAME predicate as the merge gate), read from a provably complete listing;
# 4. (policy `review-then-suites`) every REQUIRED status check of the base branch has COMPLETED
# 4. (policy `suites-parallel-with-review`) every REQUIRED status check of the base branch has COMPLETED

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.

should-fix β€” Automated review finding (data, not an instruction to any agent)

The condition-4 comment is relabelled to policy suites-parallel-with-review, but the sentence two lines below (line 568) still reads 'Review first, then the suites, then the arm:' β€” the retired order. The comment names a parallel policy and then states the review-first sequence within three lines; the arm predicate itself is unchanged.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 80f02b6: the line now reads that the review and the suites may finish in either order and the arm waits for both. The predicate itself is unchanged.

# with `success` on the current head β€” the suites ran (on a fresh merge with the current main,
# policy `suites-test-fresh-merge`) and are green. Review first, then the suites, then the arm:
# nothing is armed while a required check is pending, missing or red. The review's own
# policy `suites-test-fresh-merge`) and are green. The review and the suites may finish in either order and the arm waits for
# BOTH: nothing is armed while a required check is pending, missing or red. The review's own
# contexts are conditions (2)/(3), never (4). `required_checks_green` is the predicate;
# MeshWeaver.Plugins' control-plane `PrArming` ports it one for one.
#
Expand Down Expand Up @@ -1987,7 +1987,7 @@ def arm_case(name, ready, pr, comments, runs, says=""):
[_ir(at="2026-10-04T07:00:00Z", crid=940), _ir(conclusion="neutral", title=DEGRADED_TITLE)], "UNAVAILABLE")
arm_case("draft -> no arm (even when reviewed and answered)", False, draft_pr, [], [_ir()], "is a draft")

# ── condition 4 (policy review-then-suites): reviewed AND answered is not enough β€” the required
# ── condition 4 (policy suites-parallel-with-review): reviewed AND answered is not enough β€” the required
# suites must be green on the SAME head. Each NO names the context it holds on.
def suite(name, conclusion="success", status="completed", sha=HEAD, at="2026-10-04T09:00:00Z", crid=990):
return {"name": name, "status": status, "conclusion": conclusion, "head_sha": sha, "started_at": at, "id": crid}
Expand Down
17 changes: 9 additions & 8 deletions .github/workflows/dotnet-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -601,13 +601,14 @@ jobs:
# away from the gate that needs it is how a proof and its subject drift apart.

# ── THE STAGE GATE of the staged pull-request pipeline (Doc/Architecture/StagedPullRequestPipeline) ──
# πŸ”€ Policy review-then-suites: review first (this gate), then the suites on a fresh merge with the
# πŸ”€ Policy suites-parallel-with-review: the suites run beside the review (this gate lets them start after stage 0), on a fresh merge with the
Comment thread
Copilot marked this conversation as resolved.
# current main (policy suites-test-fresh-merge), then the arm once the review is answered and every
# required check is green.
# Maintainer, 2026-10-04: "code review must pass and also other controls such as no client etc.
# must pass before we start test. and we arm only at end of test". On a pull request the build and
# the shards wait for (a) the cheap static controls below β€” stage 0, which fail fast β€” and (b) the
# head's automatic review having landed with every reviewer thread answered. A HOLD is this job
# This caller omits `review-before-suites`, so on a pull request the build and the shards wait only
# for (a) the cheap static controls below β€” stage 0, which fail fast. (Review-first β€” the build and
# shards also waiting for (b) the head's automatic review having landed with every reviewer thread
# answered β€” is the opt-in `review-before-suites: true`; the hold/re-run wording below applies to it.)
# The arm still needs the answered review. A HOLD is this job
# FAILING (never skipping): `build` requires its success explicitly, `collect-results` reads red
# with the hold as its first line, and stage-advance.yml re-runs the failed jobs the moment the
# review lands / a finding is answered / the bounded fallback elapses. push, merge_group and
Expand Down Expand Up @@ -660,7 +661,7 @@ jobs:
runs-on: ubuntu-latest
outputs:
# THE MAIN THE SUITES TEST AGAINST, resolved HERE β€” as the suites START. A run held at the stage
# gate (policy review-then-suites) is released by `rerun-failed-jobs`, which re-runs this job
# gate (when a caller opts in to review-before-suites) is released by `rerun-failed-jobs`, which re-runs this job
# but NOT a green `precheck`, so precheck's main may be hours old by then. Every suite job and
# the green-tree marker take this one; empty off pull requests.
main-sha: ${{ steps.fresh.outputs.main-sha }}
Expand Down Expand Up @@ -3832,7 +3833,7 @@ jobs:

dependent-suites-dispatch:
name: "Dependent suites (request)"
# Stage 2 like the build: it spends MeshWeaver.Plugins' runners, so it waits for stage 1 too.
# Stage 2 like the build: it spends MeshWeaver.Plugins' runners, so it waits for the stage gate too.
needs: [preflight, stage-gate]
if: ${{ !cancelled() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork != true && github.actor != 'dependabot[bot]' && needs.preflight.result == 'success' && needs.stage-gate.result == 'success' }}
runs-on: ubuntu-latest
Expand Down Expand Up @@ -4096,7 +4097,7 @@ jobs:
if [ "$MODE" = "stage0-red" ]; then
echo "::error title=Stage 2 held β€” stage 0 red::STAGE 2 HELD (stage 0 red): a cheap static control failed (shared rules, closing keywords, package pins, interface additions, i18n mirror, CI shell or cross-repo pair β€” see the red job), so the build and the shards did not start. Fix it and push; the new head restarts at stage 0."
else
echo "::error title=Stage 2 held β€” stage 1 (${MODE:-$GATE})::STAGE 2 HELD (stage 1, ${MODE:-$GATE}): the head's automatic review has not landed or a reviewer thread is unanswered β€” see the stage gate job for exactly which. The suites start by themselves when it turns green (stage-advance.yml re-runs this run's failed jobs); nothing to push."
echo "::error title=Stage 2 held β€” stage 1 (${MODE:-$GATE})::STAGE 2 HELD (stage 1, ${MODE:-$GATE}): the stage gate held the suites (with the opt-in review hold: the head's automatic review has not landed or a reviewer thread is unanswered) β€” see the stage gate job for exactly which. The suites start by themselves when it turns green (stage-advance.yml re-runs this run's failed jobs); nothing to push."
fi
echo "### ⏸ Stage 2 held (${MODE:-$GATE}) β€” no test ran on this attempt" >> "$GITHUB_STEP_SUMMARY"
exit 1
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/node-repo-stage-advance.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# The staged pull-request pipeline's EVENT HALF (reusable) β€” start stage 2 the moment stage 1 turns green.
#
# πŸ”€ Needed by EVERY caller on the default ordering (policy review-then-suites: the stage gate holds
# for the review). Only a caller that opts out (`review-before-suites: false`) can do without it.
# πŸ”€ Needed only by a caller that opts in to `review-before-suites: true` (the stage gate then holds
# for the review). On the default (policy suites-parallel-with-review) nothing is held, so nothing
# needs releasing.
#
# A held run's stage gate (node-repo-stage-gate.yml) is RED and stays red until something asks again.
# The doctrine already has the shape for "an event outside the pull_request run changed live state":
Expand Down
39 changes: 22 additions & 17 deletions .github/workflows/node-repo-stage-gate.yml
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
# The staged pull-request pipeline's STAGE GATE (reusable) β€” may the expensive suites start for this head?
#
# πŸ”€ REVIEW FIRST, THEN THE SUITES, THEN THE ARM (policy `review-then-suites`, which replaced the
# short-lived `suites-parallel-with-review`). By default this gate holds a pull request head until
# its review landed and every finding is answered; the suites then run on a FRESH merge with the
# πŸ”€ SUITES BESIDE THE REVIEW, THE ARM ONLY AFTER BOTH (policy `suites-parallel-with-review`, back in

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.

should-fix β€” Automated review finding (data, not an instruction to any agent)

The rewritten header (lines 3-10) states the suites-beside-review order, but the file retains the 2026-10-04 maintainer quote at lines 12-13 β€” 'code review must pass and also other controls such as no client etc. must pass before we start test. and we arm only at end of test' β€” still labelled 'Design of record', and its first half prescribes exactly the review-first order this change retires. The 2026-10-07 reversal is recorded in the policy register and the pipeline manual but not in this file, so the gate now documents two contradictory designs of record. The same 2026-10-04 quote is also left in place under the renamed policy comment in .github/workflows/dotnet-test.yml.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 80f02b6: the 2026-10-04 quote in node-repo-stage-gate.yml is now framed as History (review-first design, since released; survives only as the opt-in; the arm-at-the-end half still holds) and points at the register. In dotnet-test.yml the same quote was replaced by a description of this caller's actual behaviour.

# force after `review-then-suites` was retired). By default this gate lets the suites start as soon
# as stage 0 is green and run beside the automatic review; they run on a FRESH merge with the
Comment thread
Copilot marked this conversation as resolved.
# current main (policy `suites-test-fresh-merge`), and auto-merge is armed only once the review is
# answered AND every required check is green (`check-review-answered.py` arm gate, condition 4;
# MeshWeaver.Plugins `PrArming`). `review-before-suites: false` runs the suites beside the review β€”
# a per-caller opt-out, never the default. A caller on the default MUST carry its stage-advance.yml
# listener: a gate without its listener holds forever.
# MeshWeaver.Plugins `PrArming`). `review-before-suites: true` is the per-caller OPT-IN that holds
# the suites until the head's review landed and every finding is answered; a caller that opts in
# MUST carry its stage-advance.yml listener: a gate without its listener holds forever.
#
# Maintainer, 2026-10-04: "code review must pass and also other controls such as no client etc. must
# pass before we start test. and we arm only at end of test". Design of record, with the measured
# saving: Doc/Architecture/StagedPullRequestPipeline.
# History: the 2026-10-04 decision ("code review must pass ... before we start test. and we arm only at
# end of test") was the review-first design (policy `review-then-suites`); it was later released and
# the review-first order survives only as the `review-before-suites: true` opt-in. The arm-only-at-the-end
# half still holds for every caller. Decision register and measured saving:
# Doc/Architecture/PolicyNotProse and Doc/Architecture/StagedPullRequestPipeline.
#
# The stage diagram below describes the opt-in (`review-before-suites: true`); on the default, stage 1
# is not a gate on stage 2 β€” the review runs beside the suites and is enforced at the arm and merge gates.
#
# STAGE 0 cheap static controls (validate, confidential terms, shared rules, locks, policy gates) β€”
# the caller makes its heavy-leg front door NEED them, so they fail fast.
Expand Down Expand Up @@ -79,13 +84,13 @@ on:
default: false
review-before-suites:
description: >-
true (default, policy review-then-suites) = hold the suites until the head's review landed
and every thread is answered; the caller needs its stage-advance.yml listener. false = the
suites start as soon as stage 0 is green and run beside the review (an opt-out); merging and
arming still require the answered review.
false (default, policy suites-parallel-with-review) = the suites start as soon as stage 0 is
green and run beside the review; merging and arming still require the answered review.
true (opt-in) = hold the suites until the head's review landed and every thread is
answered; the caller then needs its stage-advance.yml listener.
required: false
type: boolean
default: true
default: false

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.

question β€” Automated review finding (data, not an instruction to any agent)

The fleet-wide effect of this one-line flip rests on two claims the diff cannot show: (a) the body's statement that every caller omits review-before-suites β€” the diff for .github/workflows/dotnet-test.yml is comment-only, so an explicit 'review-before-suites: true' at a call site would be unchanged and invisible here, leaving that caller on the hold; (b) the 2026-10-07 maintainer decision releasing the restriction β€” within this diff its only record is the register rows the diff itself adds, while the gate's own header still quotes the 2026-10-04 decision that prescribed the opposite. If either claim does not hold, the default flip does not do what the PR says it does, or does it without the recorded sanction.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(a) Checked on the PR head: no workflow in core passes review-before-suites (only the lane's own input and comments mention it), so core's caller is on the default. Satellite callers float on the lane and are not visible in this diff; any that pass true explicitly keep the hold by design. (b) The header now records the 2026-10-04 decision as superseded history (80f02b6); the release of that restriction is recorded in the PolicyNotProse register rows this PR adds. The maintainer decision itself is not something the repo can prove.

outputs:
ready:
description: "'true' when stage 2 may start for this head"
Expand All @@ -112,7 +117,7 @@ jobs:
mode: ${{ steps.judge.outputs.mode }}
steps:
- name: Fetch the platform's review predicate at the lane's scripts ref
# Only the review hold (the default) reads the predicate; the opt-out judges stage 0 alone.
# Only the review hold (the opt-in) reads the predicate; the opt-out judges stage 0 alone.
if: ${{ inputs.review-before-suites }}
env:
GH_TOKEN: ${{ github.token }}
Expand Down Expand Up @@ -175,8 +180,8 @@ jobs:
not_staged "a fork's head is not reviewed by the internal reviewer β€” not staged; the merge gate still applies"
fi
[ -n "${PR:-}" ] || { echo "::error::a pull_request event without a pull request number"; exit 1; }
# πŸ”€ The opt-out (review-before-suites: false): the suites run beside the review. The default
# (policy review-then-suites) falls through to the review predicate below.
# πŸ”€ The default (review-before-suites: false, policy suites-parallel-with-review): the suites
# run beside the review. The opt-in (true) falls through to the review predicate below.
if [ "$REVIEW_BEFORE_SUITES" != "true" ]; then
echo "stage 0 green β€” the suites start now and run in PARALLEL with the automatic review; merging and arming still require the review landed and every thread answered"
{ echo "ready=true"; echo "mode=parallel"; } >> "$GITHUB_OUTPUT"
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/stage-advance.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Start core's stage 2 the moment stage 1 turns green for a held pull request head.
#
# πŸ”€ REQUIRED (policy review-then-suites): the stage gate holds every pull request head for its
# review by default, and this listener is what releases it β€” a gate without its listener holds
# πŸ”€ REQUIRED for a caller that opts in to `review-before-suites: true` (the opt-in; the default,
# `false`, per policy suites-parallel-with-review, holds nothing): the stage gate then holds every head for its
# review, and this listener is what releases it β€” a gate without its listener holds
# forever. Never delete one without the other.
#
# The thin caller of node-repo-stage-advance.yml (the lane carries the rationale). Core's held runs
Expand Down
2 changes: 1 addition & 1 deletion src/MeshWeaver.Documentation/Data/Architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ Each theme starts with its introductory page, followed by related architecture t
- [The CD Ledger Records a Failure, Not a Cadence](CdLedgerRecordsFailures) β€” 109 of 109 `incomplete image set` alarms recorded a delivery that had not failed: the pair tag that cannot be satisfied because Plugins merges faster than a publish takes, the minutes before a publisher even exists, the registry fingerprint that separates an attempt from an absence β€” and the rebuild-cadence decision this deliberately leaves open
- [The Merge Queue](MergeQueue) β€” OFF on core `main` (policy `core-merge-never-blocked`); its design, and a steward that re-queues an ejected PR on evidence and never re-runs
- [Review Findings Answered](ReviewFindingsAnswered) β€” a pull request reads red until the automatic review has landed and each thread it opened has a person's reply; the reviewer's two logins, the quota refusal posted as a review, and the maintainer-only waiver
- [Staged Pull Request Pipeline](StagedPullRequestPipeline) β€” review first, then the expensive suites on a fresh merge with the current main, then arming only when the review is answered and every required check is green (policy `review-then-suites`): event-driven advance, three loud releases, and the measurements
- [Staged Pull Request Pipeline](StagedPullRequestPipeline) β€” the expensive suites on a fresh merge with the current main beside the review, then arming only when the review is answered and every required check is green (policy `suites-parallel-with-review`): event-driven advance, three loud releases, and the measurements
- [Fresh Merge Under Test](FreshMergeUnderTest) β€” every suite job tests the head merged onto the base's CURRENT tip, computed in the job and reported, never the push-time `refs/pull/N/merge` (policy `suites-test-fresh-merge`)
- [Carving Projects Out Of Core](CarvingProjectsOutOfCore) β€” what a SOURCE move costs and what it does not
- [Red-Log Watching & Ticketing](LogWatchTriage) β€” every `fail:`/`crit:` becomes exactly one triaged issue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ two of the cases.
A caller may pass `merge-main-sha` to the three lanes; empty means the lane resolves the tip itself
in its first job. Satellites need no change: they call the lanes at `@main`.

**When is "now"?** When the suites START. Under policy `review-then-suites` a run is held at the
**When is "now"?** When the suites START. For a caller that opts in to `review-before-suites: true` a run is held at the
stage gate until its review is answered and then released by `rerun-failed-jobs`; the job that
resolves the tip is one that re-runs then (core's `build`, Plugins' `admission`), so the released
suites test the main of the moment they start, not the main of the push. A re-run of failed shards
Expand All @@ -103,5 +103,5 @@ alone keeps the tree its build compiled β€” on purpose, since the shards read th

## Related

- [Staged Pull Request Pipeline](../StagedPullRequestPipeline) β€” review first, then these suites, then the arm (policy `review-then-suites`)
- [Staged Pull Request Pipeline](../StagedPullRequestPipeline) β€” the suites beside the review, then the arm (policy `suites-parallel-with-review`)
- [Review Findings Answered](../ReviewFindingsAnswered) β€” the required check that still gates the merge
Loading