Settle the Preamble Decisions and Reshape Section 4, Section 6 and the YAML Conventions - #1395
Conversation
…e YAML Conventions Collapses pull requests 2, 4 and 6 of the #1311 sequence into one change, which the maintainer agreed to because the three touch the same surfaces and run sequentially each would re-sweep what the last swept. Applies #1206's two maintainer decisions first, since sections 4 and 6 and GOVERNANCE.md all depend on them: D9 binds exactly as D1 through D8 do, and an overlap between WORKFLOW.md and GOVERNANCE.md resolves by subject rather than by blanket precedence. Every section 1 to 6 now has a named owner. Shrinks section 4's two longest items to their outcomes, D1.6 from about 974 words and D4.5 from about 602. The displaced knowledge moves rather than being deleted: the Microsoft.Testing.Platform runner, package floor and silent-green trap to dotnet-codestyle, the pytest-cov dependency and coverage selector to python-codestyle, and the failed-push recovery routes to operational-vs-release-workflow. Restores D6.2's publisher exception, which #1394 removed with 5A, keyed on the entry-point count rather than on a job name or a missing input, both of which a withdrawn attempt disproved. Rebuilds section 6 so it states only what each type adds, deleting all eight per-type N/A lists rather than correcting them, because such a list is right for a single-type repo and negates a construct a second declared type supplies. Section 1's applicability rule derives the N/A set instead, so no row can take away what another row gives. Splits upstream-wrapper out of the docker row. Sweeps GOVERNANCE.md "Workflow YAML Conventions" for the items on #1367. Refs #1311, #1206, #1367, #1369, #1370, #1371, #1393 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Disposes of three review rounds over this change: 21 read-only passes on the strongest tier, 11 whole-unit plus a cross-unit diff pass in round one, 9 in round two, and a terminal pair scoped to false claims and unfollowable instructions. Two maintainer decisions. D6.2's publisher exception is withdrawn and the item returns to the text develop carries, a fourth wording having been disproved by get-version-task.yml, a called workflow whose branch classification comes from the caller's GITHUB_REF exactly as D3.1 requires. The bullet-to-D-item mapping this change had added to GOVERNANCE.md and to WORKFLOW.md section 2 is deleted, having been wrong four ways and having drifted between its two copies in the round that wrote it. Section 6's construct table loses the column that keyed constructs on releaseTrigger and on type names, which gave every releaseTrigger: none repo two answers on whether it has a publisher. A construct is present or it is not, and the reader looks. Also removes a sentence duplicated verbatim in 5B's preamble by an earlier revert in this branch, corrects five claims about GitHub Actions behaviour that the tree falsifies, and rehomes what the shrink displaced so no pointer dead-ends. Records a carried-content pass over all 23 canonical units this change moves. Refs #1311, #1206, #1367, #1369, #1370, #1371, #1393 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟡 Changes recommended
It introduces markdown-formatting convention violations (hard-wrapped paragraphs) and one newly added sentence in WORKFLOW.md that is misleading as written.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR consolidates and applies the agreed “preamble decisions” and restructures the CI/CD contract documentation by tightening WORKFLOW.md sections 4 and 6 and sweeping GOVERNANCE.md’s “Workflow YAML Conventions”, with supporting detail moved into the relevant skills and references (and the generated distributions regenerated).
Changes:
- Refines
WORKFLOW.md’s framing, applicability rules, D-guarantee preamble, and reshapes section 6 around “constructs each type adds” instead of per-type N/A lists. - Sweeps and clarifies
GOVERNANCE.md“Workflow YAML Conventions” to align with the updated contract framing and exceptions. - Updates skill/reference content (and regenerates
.github/skills/+.claude-plugin/copies) to relocate detailed mechanics (e.g., MTP + coverage, publish recovery) out ofWORKFLOW.md.
File summaries
| File | Description |
|---|---|
| WORKFLOW.md | Reshapes preamble, contract framing, and section 6 type/construct mapping. |
| GOVERNANCE.md | Sweeps “Workflow YAML Conventions” to match subject-based overlap and clarified rules. |
| CODESTYLE.md | Updates section summaries to point detailed coverage/MTP guidance into codestyle skills. |
| AUDIT.md | Clarifies “outcome vs verbatim” and how named constructs affect audit verdicts. |
| reports/canonical-review.json | Records refreshed canonical-unit review coverage entries. |
| .agents/skills/workflow-ci-contract/SKILL.md | Updates skill text to reflect section 6’s new “constructs added” framing. |
| .agents/skills/workflow-ci-contract/references/architecture.md | Mirrors updated architecture wording from WORKFLOW.md section 3. |
| .agents/skills/workflow-ci-contract/references/d-guarantees.md | Mirrors updated D-guarantee preamble text from WORKFLOW.md section 4. |
| .agents/skills/workflow-ci-contract/references/test-methodology.md | Mirrors updated 5B preamble guidance from WORKFLOW.md section 5. |
| .agents/skills/python-codestyle/references/testing.md | Expands Python coverage obligations (pytest-cov + selector) per D1.6 move. |
| .agents/skills/dotnet-codestyle/SKILL.md | Notes MTP-based test project requirements at the skill summary level. |
| .agents/skills/dotnet-codestyle/references/testing.md | Adds detailed MTP + coverage requirements and diagnostics guidance. |
| .agents/skills/operational-vs-release-workflow/SKILL.md | Adds summary bullet for “failed registry push after release cut” recovery. |
| .agents/skills/operational-vs-release-workflow/references/release-publish-mechanics.md | Adds detailed recovery procedure for failed post-release registry push. |
| .github/skills/workflow-ci-contract/SKILL.md | Regenerated distribution copy of the workflow-ci-contract skill. |
| .github/skills/workflow-ci-contract/references/architecture.md | Regenerated distribution copy of workflow-ci-contract architecture reference. |
| .github/skills/workflow-ci-contract/references/d-guarantees.md | Regenerated distribution copy of workflow-ci-contract D-guarantees reference. |
| .github/skills/workflow-ci-contract/references/test-methodology.md | Regenerated distribution copy of workflow-ci-contract test methodology reference. |
| .github/skills/python-codestyle/references/testing.md | Regenerated distribution copy of python-codestyle testing reference. |
| .github/skills/dotnet-codestyle/SKILL.md | Regenerated distribution copy of dotnet-codestyle skill. |
| .github/skills/dotnet-codestyle/references/testing.md | Regenerated distribution copy of dotnet-codestyle testing reference. |
| .github/skills/operational-vs-release-workflow/SKILL.md | Regenerated distribution copy of operational-vs-release-workflow skill. |
| .github/skills/operational-vs-release-workflow/references/release-publish-mechanics.md | Regenerated distribution copy of publishing mechanics reference. |
| .claude-plugin/fleet-skills/skills/workflow-ci-contract/SKILL.md | Regenerated Claude plugin copy of workflow-ci-contract skill. |
| .claude-plugin/fleet-skills/skills/workflow-ci-contract/references/architecture.md | Regenerated Claude plugin copy of architecture reference. |
| .claude-plugin/fleet-skills/skills/workflow-ci-contract/references/d-guarantees.md | Regenerated Claude plugin copy of D-guarantees reference. |
| .claude-plugin/fleet-skills/skills/workflow-ci-contract/references/test-methodology.md | Regenerated Claude plugin copy of test methodology reference. |
| .claude-plugin/fleet-skills/skills/python-codestyle/references/testing.md | Regenerated Claude plugin copy of python testing reference. |
| .claude-plugin/fleet-skills/skills/dotnet-codestyle/SKILL.md | Regenerated Claude plugin copy of dotnet skill. |
| .claude-plugin/fleet-skills/skills/dotnet-codestyle/references/testing.md | Regenerated Claude plugin copy of dotnet testing reference. |
| .claude-plugin/fleet-skills/skills/operational-vs-release-workflow/SKILL.md | Regenerated Claude plugin copy of operational/release skill. |
| .claude-plugin/fleet-skills/skills/operational-vs-release-workflow/references/release-publish-mechanics.md | Regenerated Claude plugin copy of publishing mechanics reference. |
| .claude-plugin/fleet-skills/.source-digests/workflow-ci-contract | Updates Claude plugin source digest for workflow-ci-contract. |
| .claude-plugin/fleet-skills/.source-digests/python-codestyle | Updates Claude plugin source digest for python-codestyle. |
| .claude-plugin/fleet-skills/.source-digests/dotnet-codestyle | Updates Claude plugin source digest for dotnet-codestyle. |
| .claude-plugin/fleet-skills/.source-digests/operational-vs-release-workflow | Updates Claude plugin source digest for operational-vs-release-workflow. |
Review details
Suppressed comments (1)
.agents/skills/dotnet-codestyle/references/testing.md:45
- These newly added paragraphs are hard-wrapped mid-sentence. Repo Markdown convention is one logical paragraph per line (no hard-wrap); see
.github/skills/comment-and-doc-style/SKILL.md:90-92.
**The version floor is load-bearing rather than cautionary.** Below 18.1.0 the extension is built against Microsoft.Testing.Platform 1.x,
and an 18.0.x resolution, which is what a `>= 18.0.0` range picks, throws a `TypeLoadException`
against the 2.x platform `xunit.v3` 4.0.0 carries, runs zero tests, and **still writes a
well-formed Cobertura file reporting full coverage**, so only the non-zero exit says the run
reported nothing. 18.9.0 is the first release on Microsoft.Testing.Platform 2.3.x, where every
- Files reviewed: 36/36 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Answers Copilot's round 1 on #1395. The prose this change added to three skill reference files was hard-wrapped, against the fleet rule that a logical paragraph is one line with no length limit. It is unwrapped. The sibling reference files wrap the same way and are left alone, since new content respecting a rule the rest of the tree has yet to meet is what the target-state framing means. Section 6's intro claimed each scenario is reached by exactly one construct, which S1 falsifies by needing both the pull request workflow and a build target. Filing a scenario under one row is what covers the set; applicability takes every construct the scenario needs. Records a carried-content pass over the four units this moves. Refs #1311 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the suppressed finding, which has no thread to resolve.
The sibling reference files across the skills tree wrap the same way and are deliberately untouched. Bringing new content up to a rule the rest of the tree has yet to meet is exactly what |
There was a problem hiding this comment.
🔵 Needs a closer look
One updated prose line in GOVERNANCE.md uses a mid-sentence semicolon that conflicts with the repo’s no-semicolons-in-prose convention (and is likely to fail prose_lint).
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
GOVERNANCE.md:238
- Semicolon rule: replace the mid-sentence semicolon with a comma or sentence break to comply with the repo's no-semicolons-in-prose convention (prose_lint).
- Files reviewed: 36/36 changed files
- Comments generated: 0 new
- Review effort level: Lite
Answers Copilot's round 2 on #1395, whose one finding was a semicolon in the artifact-retention bullet this change rewrote. The semicolon joined two independent clauses, which the rule bans. prose_lint passed it because the same sentence carries an unrelated colon, and a colon announcing a list exempts every semicolon after it within that sentence. The exemption is right for an enumeration whose items carry commas and wrong here, where the colon introduces an explanation rather than a list. The rule's home already records fixing this shape at bullet scope, so the residual gap is filed rather than fixed alongside a prose change. Refs #1311 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering round 2's suppressed finding, which has no thread to resolve.
That is the same over-exemption |
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces at least two documentation assertions whose current wording is internally inconsistent or contradicted by existing repo state.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
GOVERNANCE.md:232
- This rule states that a POSIX
#!/bin/shgit-hook surface "takesset -eu", but this repository’s own POSIX hook.husky/pre-commitcurrently usesset -e(see.husky/pre-commit:15). Either the hook needs to be updated in this PR, or this text should be loosened so the conventions section isn’t contradicted by the tree it governs.
WORKFLOW.md:300 - This bullet says ownership vs calling "decides which constructs it contains", but the very next clause explains both an owner and a caller still contain the D6.1/D5 constructs (they just live in different places for citation). Rewording avoids an internal contradiction and prevents readers from concluding that calling the hub task makes these constructs N/A.
- Files reviewed: 36/36 changed files
- Comments generated: 0 new
- Review effort level: Lite
Answers Copilot's round 3 on #1395, whose two findings both stand. The Shells convention this change states requires set -eu of a POSIX /bin/sh surface, and this repository's own pre-commit hook carried set -e, so the section was contradicted by the tree it governs. The hook takes set -eu. It references no variable and deliberately does not source the generated _/husky.sh, so -u adds no failure path. The adopter-facing catalog snippet does source that file and carries no set line at all, and stays filed rather than changed against a file this repository cannot inspect. Section 6's source-only row said ownership of the release task decides which constructs a repo contains, which the reading rule above it contradicts and which would have a caller record D6.1 and D5.1 to D5.5 N/A. Both own and call contain them, and ownership changes only where 5A cites the evidence. Refs #1311, #1367 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering round 3's two suppressed findings, which have no threads to resolve. Both stand and both are fixed in 11804dd.
The adopter-facing
|
There was a problem hiding this comment.
🔵 Needs a closer look
It reshapes multiple fleet-defining contract documents and regenerated skill distributions, so it warrants final human verification of correctness and downstream impact.
Review details
- Files reviewed: 37/37 changed files
- Comments generated: 0 new
- Review effort level: Lite
…sh the WORKFLOW.md Reshape (#1397) Closes #1205. Closes #1212. Closes #1240. Closes #1250. Closes #1267. Closes #1268. Closes #1271. Closes #1288. Closes #1305. Closes #1314. Sixteen commits, ten issues. Each was driven as its own feature pull request into `develop`, reviewed by the PR-hosted reviewers, and merged only with CI green and every finding disposed of by one of the five outcomes: fixed, declined on evidence, decided by the maintainer, deferred behind a filed issue, or fixed as a class. ## What this promotes **The one-home include mechanism and its first six classes** (#1317). `scripts/build_dist.py` gained include regions filled from a rule's home and checked by `--check` (#1378), so a Skill carries a rule's whole text without a copy that can drift. Classes 2 to 6 then converted the restatements: `agent-conduct`'s three conduct sections (#1382), `pr-review-conduct`'s five outcomes into `drive-pr` with every step-ref renamed to a heading (#1383), `backlog-burndown`'s two narrowing rows cut to the narrowing with fourteen restatements pointered (#1384), `WORKFLOW.md` section 4 into `workflow-ci-contract` (#1385), and section 2 cut to a pointer at `GOVERNANCE.md` "Workflow YAML Conventions" (#1388). **The `WORKFLOW.md` reshape** (#1311 step 14's six-pull-request sequence, now finished). The verdict clause aligned with section 5's Assessment (#1390), sections 3 and 5 carried into `workflow-ci-contract` as generated includes (#1392), 5A collapsed to a procedure and an evidence rule (#1394), and the preamble decisions settled alongside the reshape of section 4, section 6 and the YAML conventions (#1395). Section 4's two longest items shrank to their outcomes with the displaced knowledge moved rather than deleted, and section 6 now states only what each type adds, carrying no N/A list at all. **The review loop's stop rule and disposition policy** (#1330), rewriting disposal by deletion and committing the condition under which a whole-unit loop ends, which #1267 filed as missing. **The Merge Gate's bound on an out-of-diff prose finding**, with the reviewer footing recorded (#1333), and reviewer bots scoped away from the generated Skill mirrors (#1329) so a mirror's diff is never reviewed in place of its source. **The fleet label set**, declared and applied through `configure.sh` (#1334). **The review ledger and skills digest decoupled from the working tree** (#1328), so concurrent branches no longer conflict in a generated report that cannot be hand-merged. Plus one grouped Dependabot bump, `docker/setup-qemu-action` 4.2.0 to 4.3.0 (#1325). ## What is deliberately not closed `#1311`, `#1317`, `#1206`, `#1367`, `#1369`, `#1370`, `#1371`, `#1386` and `#1237` each still hold findings this work did not settle. #1317 stands at class 6 of fourteen, and #1311's step 17 comment records what the reshape filed rather than fixed. ## Owed on merge `spec/files.json` declares both edited `GOVERNANCE.md` sections at `verbatim` fidelity, so every downstream copy goes stale on this promotion and a fleet resync follows it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Collapses pull requests 2, 4 and 6 of the #1311 sequence into one change, which the maintainer
agreed to because the three touch the same surfaces and, run sequentially, each would re-sweep what
the last swept and pay the seam cost three times.
What it does
#1206's two maintainer decisions, applied first, since sections 4 and 6 and
GOVERNANCE.mdalldepend on them. D9 binds exactly as D1 through D8 do, so a style violation is a defect and
GOVERNANCE.md's target-state framing settles when an unswept workflow is fixed rather thanwhether the violation counts. An overlap resolves by subject rather than by blanket
precedence, and every section 1 to 6 now has a named owner.
Section 4's two longest items shrink to their outcomes, D1.6 from about 974 words and D4.5 from
about 602. Nothing is deleted: the Microsoft.Testing.Platform runner, package floor and its
silently-green failure modes move to
dotnet-codestyle, thepytest-covdependency and coverageselector to
python-codestyle, and the failed-push recovery routes tooperational-vs-release-workflow.Section 6 states only what each type adds, and carries no N/A list at all. All eight per-type
N/A lists are deleted rather than corrected, because such a list is right for a single-type
repository and negates a construct a second declared type supplies, which is the defect #1394
shipped through fifteen clean unit passes. Section 1's applicability rule derives the N/A set
instead, so no row can take away what another row gives. Rows are added for the seven registry
types that had none,
upstream-wrapperis split out ofdocker, and the construct table keys onthe construct being present rather than on a registry field.
GOVERNANCE.md"Workflow YAML Conventions" is swept for the items on #1367.What is deliberately not done
disproved by
get-version-task.yml, a called workflow whose branch classification comes from thecaller's
GITHUB_REFexactly as D3.1 requires. The item returns to the textdevelopcarries andTracker:
WORKFLOW.md > 4. Behavioral Contract: Expected Outcomes#1369 records all four disproofs.two copies in the round that wrote it, and was deleted on the maintainer's call.
pypienvironment's deployment-branch rule is not restored, per the caveat Tracker:WORKFLOW.md > 5. Test Methodology#1370 records:its one plausible content would block every
.dev0publish D3.4 requires.Review evidence
21 read-only passes on the strongest tier before this pull request existed: 11 in round one (10
whole-unit carried-content plus a cross-unit diff pass briefed to hold
registry/repos.jsonopen),9 in round two, and a terminal pair scoped to false claims and unfollowable instructions. Roughly
220 findings, the large majority introduced by this change and disposed of in it. The terminal pass
over
GOVERNANCE.md,AUDIT.md,CODESTYLE.mdand the four skills returned zero.The measurement worth recording: the deletions were right first time and the reasons drew nearly
everything. Five claims about GitHub Actions behaviour that this change introduced were falsified
by the tree and reverted, among them that
always()and!failure() && !cancelled()areinterchangeable-with-a-preference (D1.5 requires
always()for the aggregator), and that theregistry's
releaseTriggerdecides publish-scenario applicability (it saystwo-phasefor thisrepository while
publish-release.ymlisworkflow_dispatch:only).A carried-content pass is recorded for all 23 canonical units this change moves.
Owed on merge
spec/files.jsondeclares both editedGOVERNANCE.mdsections atverbatimfidelity, so everydownstream copy goes stale and a fleet resync follows.
🤖 Generated with Claude Code