Cut WORKFLOW.md Section 2 to a Pointer at Workflow YAML Conventions - #1388
Conversation
Class 6 of #1317. WORKFLOW.md section 2 restated GOVERNANCE.md "Workflow YAML Conventions" as eleven condensed bullets, twelve rows in the inventory, with a canonical-scope note declaring which copy wins. The section keeps its heading and number and becomes a pointer at the home, and its line-endings bullet, whose home is the policy "Documentation Style Conventions" routes to, becomes a pointer too. The five sentences that cited section 2 for a rule name the home instead: the preamble's audit verb and canonical-scope note, the D9 preamble inside section 4, the D8/D9 walkthrough in 5A, and the operational bullet in section 6. GOVERNANCE.md's section stops claiming two homes, the skill's Style Rules points at the home and at the line-ending policy's skill, and the generated include and distributions regenerate. The ledger records a whole-unit pass over each of the eight units this moves. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ss 6 The first diff pass over class 6 found two cross-file defects the whole-unit reads could not see. The canonical-scope note and the GOVERNANCE.md section had lost the sentence saying GOVERNANCE.md wins wherever WORKFLOW.md overlaps it, while sections 3 and 4 still restate that section's bullets and D7.3 contradicts one of them, so the tie-break is restored unscoped rather than narrowed to section 3. The AGENTS.md "Where the Rules Live" row said this section and WORKFLOW.md keep the full rules, and now says the section keeps the style rules and WORKFLOW.md the contract. A scoped third round under the stop rule, on the maintainer's grant in this session, with the three moved units read whole again and recorded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…L LF Rule The second diff pass over class 6 found the 5A walkthrough citing D9.5 for .gitattributes, which D9.5 does not name, and found that the section 2 bullet the class cut, "Workflow YAML is LF (Actions and Dependabot rewrite it that way)", had its only home there. A grant moved that rule into the line-endings policy, and the whole-unit reads that move owed showed its reason false under the configuration the policy prescribes: with the paired eol=crlf attribute git stores LF and checks out CRLF, so a Dependabot commit leaves nothing mixed, and Actions never writes a workflow file. On the maintainer's call in this session the rule is retired rather than carried without its reason, the policy returns to its develop text, and the ledger keeps the passes recorded over the two policy sections as they stood on this branch. Co-Authored-By: Claude Fable 5.1 <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
The updated WORKFLOW.md preamble includes ungrammatical/unclear phrasing that should be corrected before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR converts WORKFLOW.md section 2 from a full restatement of workflow style rules into a pointer to the canonical home in GOVERNANCE.md, and updates related references across the workflow contract surfaces and skill distributions to match.
Changes:
- Replace
WORKFLOW.mdsection 2’s bullet list with a pointer toGOVERNANCE.md"Workflow YAML Conventions" (and to the line-ending policy route). - Update
WORKFLOW.mdD9 and other references to citeGOVERNANCE.mdas the source of the style-rule exceptions and suffix rules. - Regenerate/refresh the carried skill distributions and canonical-review ledger entries reflecting the updated units.
File summaries
| File | Description |
|---|---|
| WORKFLOW.md | Converts section 2 into a pointer and updates D9 / walkthrough references accordingly. |
| GOVERNANCE.md | Updates the "Workflow YAML Conventions" preamble sentence to reflect WORKFLOW.md section 2 as a pointer. |
| AGENTS.md | Updates the "Editing a workflow YAML file" routing row to reflect the new ownership split (style rules vs contract). |
| reports/canonical-review.json | Updates the canonical review ledger entries/digests and stamps for the touched units. |
| .agents/skills/workflow-ci-contract/SKILL.md | Updates the skill’s "Style Rules" pointer text to reflect the new ownership and line-ending policy route. |
| .agents/skills/workflow-ci-contract/references/d-guarantees.md | Updates D9’s pointer sentence to cite GOVERNANCE.md instead of WORKFLOW.md section 2. |
| .github/skills/workflow-ci-contract/SKILL.md | Regenerated mirror of the skill text reflecting the updated pointer. |
| .github/skills/workflow-ci-contract/references/d-guarantees.md | Regenerated mirror of the D9 pointer sentence change. |
| .claude-plugin/fleet-skills/skills/workflow-ci-contract/SKILL.md | Regenerated plugin mirror of the skill text reflecting the updated pointer. |
| .claude-plugin/fleet-skills/skills/workflow-ci-contract/references/d-guarantees.md | Regenerated plugin mirror of the D9 pointer sentence change. |
| .claude-plugin/fleet-skills/.source-digests/workflow-ci-contract | Updates the source digest for the workflow-ci-contract skill bundle. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…numeration Copilot's first round on the class 6 pull request raised two threads on the preamble: "the style conventions section 2 points at" read without a relative pronoun in the defining-principle paragraph and again in the Audit bullet, and the opening sentence named no section for the style rules' home. The pronoun is added in both places. The opening keeps naming the document rather than one section, because section 2 routes to two of them, and the canonical-scope note keeps naming "Workflow YAML Conventions" alone, since a clause naming the line-endings section drew a finding on each read for saying the rule lives where the section only routes to a Skill. The tie-break states GOVERNANCE.md as authoritative wherever this document overlaps it, without enumerating sections, on the maintainer's grants in this session. The preamble is read whole again and recorded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
A few newly introduced sentences are ambiguous/run-on in the contract docs and should be clarified to avoid misrouting readers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
WORKFLOW.md:3
- The opening paragraph uses both "style rules" and "Code style" back-to-back, but the first phrase is about workflow YAML conventions (in GOVERNANCE.md) while the second points at CODESTYLE.md. Rewording to explicitly say "workflow YAML style conventions" avoids implying CODESTYLE.md and GOVERNANCE.md both own the same kind of style rules.
The guide for CI/CD **workflows** (GitHub Actions): a deliberate mixture of architecture, a **behavioral contract** (expected inputs and outputs), and a **test methodology**, the style rules having their home in `GOVERNANCE.md`. Code style lives in [`CODESTYLE.md`][codestyle]. This file is its sibling for everything under `.github/workflows/`.
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Lite
Copilot's second round on the class 6 pull request raised three wording findings on text this change wrote, and the whole-unit reads those fixes owed raised a fourth. The "Workflow YAML Conventions" intro sentence shifted subject three times and is now two sentences, the section keeping the rules and winning on overlap in one, the Skill surfacing it in the other. Section 2's "the line-ending policy every other file takes" read as excluding workflow YAML and now says "the same line-ending policy as every other file". The opening's "style rules" beside "Code style" read as two owners of one kind of rule and now says "workflow style rules". Section 2's closing clause, reworded earlier from a parenthetical into a "since" clause, fixed the property its "necessary but not sufficient" claim is measured against and made that claim false, so develop's own parenthetical is restored verbatim. The three moved units are read whole again and recorded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the suppressed low-confidence finding from the round 2 review on head
Fixed in 571e504. The opening now reads "the workflow style rules having their home in Round 1's suppressed block was empty and round 2's is the one above, so no earlier-round suppressed finding stands unanswered. |
There was a problem hiding this comment.
🔵 Needs a closer look
Two updated doc sentences still contain a factual section-mapping mismatch and a table-row grammar break that should be corrected before merge.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
AGENTS.md:100
- The table row reads as a sentence fragment ("..., the
workflow-ci-contractSkill, that section keeping...") because it is missing a connector before "that section". Adding "with" makes the row parse cleanly and avoids ambiguity about what keeps which rules.
WORKFLOW.md:13 - The canonical-scope note says the document is authoritative for the workflow contract and test methodology "(sections 3 to 6)", but section 3 is Architecture, section 4 is the Behavioral Contract, and the test methodology is sections 5-6. As written, this sentence mislabels the section mapping and makes it harder to follow the intended split of authority.
- Files reviewed: 11/11 changed files
- Comments generated: 0 new
- Review effort level: Lite
Copilot's third round on the class 6 pull request raised no new thread and two suppressed findings on the head. This is the one on text this change wrote: the "Editing a workflow YAML file" row read as a fragment, since the clause naming which document keeps what followed the Skill with no connector. It now reads "with that section keeping the style rules and WORKFLOW.md the contract", the reviewer's own proposed fix. The other suppressed finding, the canonical-scope note's "(sections 3 to 6)" mislabelling section 3, is byte-identical on develop and is #1206's own claim, answered in the pull request rather than fixed here. The moved unit is read whole again and recorded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the two suppressed low-confidence findings from the round 3 review on head
Fixed in 395024c, with the reviewer's own proposed connector. The row now reads "surfaced with the full
No change needed here, and the finding is correct rather than wrong. The parenthetical is byte-identical on This pull request rewrote the two sentences after it and left that clause untouched, so the mislabel predates the change. It is already filed as its own item on #1206, the open Both round 2 suppressed findings were answered earlier in this conversation, so nothing from an earlier round stands unanswered. |
There was a problem hiding this comment.
🔵 Needs a closer look
The canonical-scope sentence in WORKFLOW.md still describes “contract and test methodology” as spanning sections 3-6 even though section 3 is Architecture; the fix is a small wording correction on an edited line.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
WORKFLOW.md:13
- The canonical-scope sentence says the workflow contract and test methodology are in "sections 3 to 6", but section 3 is the Architecture section. Rewording the subject list to include architecture makes the section range accurate and avoids repeating the mismatch called out elsewhere in the doc.
- Files reviewed: 11/11 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Answering the suppressed low-confidence finding from the round 4 review on head
No change needed here. This is the same finding the round 3 block raised, reworded, and it is correct rather than wrong. It is declined again for the same reason, put to the maintainer this time because a second raise makes it a recurring finding rather than a one-off, and answered here so the head carries no unanswered suppressed finding. The clause is byte-identical on It is filed on #1206, the open The reviewer's own note that the mismatch is "called out elsewhere in the doc" is the reason to settle it there rather than here: this class replaced |
Correction to this pull request's read recordThree errors in my own reporting, found after the merge. None changes a finding, a fix, or the merge decision, and all three are about how the reads were briefed and counted. 1. Nine of thirteen preamble findings in rounds 1 to 3 re-derived claims already filed. Those three rounds were briefed "None filed for this unit, tracker None", and #1206 existed throughout holding about eleven claims about that exact preamble. I found #1206 only later, while searching for issues about section 2. The overlap, by round:
The four that were not re-derivations: the "operational" verdict word and the tie-break's silence on a Nothing was double-filed. I mapped the overlap before filing and the #1206 comments name which findings restate its claims. What the record overstates is discovery: those rounds' 2. Tracker #1386 was fetched with 3. "The retirement left two ledger entries" is wrong; it left one. What this does not touch. Every unit read covered its whole current text, which is what the carried-content pass requires, and an absent suppression list cannot make a raised finding false. The |
…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)
Class 6 of #1317,
WORKFLOW.mdsection 2 againstGOVERNANCE.md"Workflow YAML Conventions", on the maintainer's call in this session: pointer, not include, since the include generator walks only.agents/skills/and an include intoWORKFLOW.mdwould need an engine change first.What changed
GOVERNANCE.md"Workflow YAML Conventions" and nothing of its content. The line-endings bullet had no home in that section, its home being thecomment-and-doc-styleSkill's policy thatGOVERNANCE.md"Documentation Style Conventions" routes to under "Line Endings", so it becomes a pointer at that route rather than a rule moved into the YAML section. The row that contradicted D7.3, the both-forms boolean comparison, goes with the bullets, and the conflict it exposed is now the home's own, on Tracker:GOVERNANCE.md > Workflow YAML Conventions#1367 item 1 and Tracker:WORKFLOW.md > 4. Behavioral Contract: Expected Outcomes#1369 item 35.references/d-guarantees.mdand the three distributions regenerate from the D9 sentence.GOVERNANCE.md's section no longer says it andWORKFLOW.mdkeep the full rules with itself winning on overlap, and the skill's "Style Rules" points at the home and at the line-ending policy's skill instead of at section 2.* text=auto eol=crlfgit stores LF and checks out CRLF, so a Dependabot commit of LF bytes leaves nothing mixed, and Actions never writes a workflow file. On the maintainer's call the rule is deleted rather than carried without its reason, and the policy file is back at itsdeveloptext. The fleet's one CRLF config repo carries such a pin by hand, with the same reason in its comment, and that is left to the maintainer.local-strict-review"The Carried-Content Pass".What did not change
The "Release Model" overlap in the canonical-scope note stays, since it is a section 3 row set the inventory lists under that home, not this class. D9's five style guarantees in section 4 stay as written, since section 4 rows were not in this class and D9 states outcomes a workflow must meet rather than the conventions' text.
README.md's "Conventions" list is class 13. Two phrases the condensed bullets carried and the home does not go with them: "Lowercase, hyphen-separated" on the Filename bullet, which the inventory row already listed as drift the condensation added to a suffix-only rule, and "Grant least privilege" on the permissions bullet, which D7.2 states as the one entry point that needs the scope. The third diff pass raised the first as a rule retired without a home, and it is declined on the inventory's evidence rather than promoted into the home, since adding a case rule toGOVERNANCE.mdis class 14's decision list. #1369 items 35 and 50 name "section 2" for a rule that now lives at the home alone, noted on that tracker rather than edited.Verification
python3 scripts/build_dist.py --check,python3 scripts/prose_lint.py . --diff origin/develop,python3 scripts/repo_gate.py --check eol,python3 spec/validate.py,python3 scripts/tests/test_build_dist.py, and markdownlint, editorconfig-checker, and cspell throughscripts/docker_lint.py: clean.fabletier through round 10 and at theopustier from round 11, when that tier stopped being reachable mid-drive, recorded inreports/canonical-review.json, two of them the line-endings policy sections no pass had read before, which the final diff no longer touches. Three diff passes perlocal-strict-review, one after each commit, the last recorded as the receipt.Read record,
introducedover total per readEight units in round 1, then only the units the previous round's fixes changed. Rounds 1 and 2 were the edit budget, round 3 is the read the round 2 fix owed, and rounds 4 to 6 are the maintainer's scoped grants under the stop rule, the first two answering the diff pass before them, the third and fourth answering what the grants' own reads found, the fourth a deletion, rounds 7 and 8 are the reads owed by the fix for Copilot's first round, the second push's two-round budget, and round 9 is the maintainer's fifth grant, un-enumerating the tie-break after the round 8 read found the three-section form naming no winner for a conflict with the Skill text a named section routes to, its Release Model half being #1206's own loop, and round 10 the sixth, dropping the line-endings clause the Copilot fix had added to the scope note after the round 9 read found "live in" false for a section that only routes to a Skill. The diff pass row sits in the column of the round it followed.
The introduced findings, all on the pointer prose. Round 1: the Style Rules sentence said
GOVERNANCE.md"Line Endings" keeps the line-ending policy where that subsection only routes to thecomment-and-doc-styleSkill, the same shape in section 2 and 5A, fixed as one class, and the preamble still called the document a mixture that includes code style. Round 2, both on the preamble sentences round 1 rewrote: "theGOVERNANCE.mdsection it points at" named one section where section 2 routes to two and 5C needs a third, and "pointed at rather than carried" was false because D9 carries five style guarantees. Round 3 raised four pre-existing findings and nothing introduced. The first diff pass then raised two introduced findings on surfaces no whole-unit read holds: theAGENTS.md"Where the Rules Live" row still said this section andWORKFLOW.mdkeep the full rules, and the two rewritten scope sentences had dropped the tie-break sayingGOVERNANCE.mdwins wherever the two overlap, narrowing the release one to section 3 while sections 3 and 4 still restate that section's bullets and D7.3 contradicts one, an added qualifier where the original had none. Both fixed in the first granted round, the tie-break restored unscoped. The second diff pass raised two more: the cut line-endings bullet, "Workflow YAML is LF", had its only home in section 2, since the line-endings policy's operational section requires a CRLF default in both files and never exempts workflow YAML while the fleet's one CRLF config repo pins its workflows to LF by hand, and the 5A sentence cited D9.5 for.gitattributes, which D9.5 does not name. The second grant moved the rule into that policy, its "Operational (config) repos" section stating the pin and its reason and "Scripts and extensionless executables" no longer saying the LF overrides are for POSIX-executed scripts only, and dropped the qualifier. The reads of the two policy sections that grant wrote raised three introduced findings on the granted sentences: the pin was written as a brace glob.gitattributescannot expand, the operational sentence read as the only LF carve-out, and the scripts-section pointer claimed its sibling holds the POSIX-script exception. The third grant rewrote both sentences, and the re-read then showed the pin's reason itself false under the paired attribute, which the fourth grant answered by retiring the rule and returning the policy to itsdeveloptext, so the two policy sections are not in the final diff and the ledger's two entries for them record passes over text this branch held and no longer does. The granted round's three reads raised seven pre-existing findings and nothing introduced, the restored tie-break judged right on its own terms with the D7.3 conflict it overrules left at the home. One of round 3's four, that "keep workflows legible" mischaracterizes a section that is mostly safety rules, sits on a sentence this change re-routed, and it is classed pre-existing because the claim is #1206's own second item, filed before this change, the edit having touched only the route.The pre-existing findings, 54 in all, are filed: #1336 items 2 to 9, #1367 items 9 to 14, #1369 items 52 to 57, #1370 items 20 to 34, #1371 items 2 to 4, #1389, the tracker this class opens for the line-endings reference, items 1 to 5, and the preamble's on #1206, the open decision issue that already holds most of them, which gains the six it did not. Section 5 widened from 7 to 6 new findings on unchanged text across two reads, and the preamble from 4 to 2 to 4, the pattern #1385 measured.
This pull request closes no issue. It removes two of #1206's preamble claims, the "restates them" sentence and the undrawn code-style boundary, and leaves that decision issue open.
Copilot ran four rounds, one per head, with full coverage on each. Its first round raised two threads: a missing relative pronoun in two sentences, fixed in 1a8b243, and the opening sentence not naming the style home's section, answered in 1a8b243 by keeping the opening at the document rather than one section, since the round 7 read showed one named section to be one home where section 2 routes to two, and the scope note's attempt to name both drew a finding on each of the next two reads, so it names "Workflow YAML Conventions" alone as it did when rounds 3, 4, and 6 read it clean. Round 10 then raised that one-section form as omitting the line-endings home, the mirror of round 7, and it is classed pre-existing because the same sentence shape and omission stood on
developand #1206 already holds section 2's line endings against the overlap partition. The two readings are each right about the sentence and cannot both be satisfied by naming sections, which is the non-convergence #1385 measured, and the home question is #1206's to settle. Its second round raised two threads and one suppressed finding, all wording on sentences this change wrote, fixed in 571e504: theGOVERNANCE.mdintro sentence split in two, section 2's "every other file" reworded, and "workflow style rules" in the opening. Round 11 read the three units those fixes moved and found one more on this change's own text, section 2's closing clause reworded earlier from a parenthetical into a "since" clause, which fixed the property its "necessary but not sufficient" claim is measured against and made the claim false, sodevelop's parenthetical is restored verbatim and round 12 read it clean. Its third round raised no thread and two suppressed findings, both on unchanged text: the rules-map row reading as a fragment, fixed in 395024c with the reviewer's own connector, and the canonical-scope note's "(sections 3 to 6)" mislabel, answered in the pull request as byte-identical ondevelopand already #1206's item. Round 13 read the map whole and raised five more, none introduced. Its fourth round raised no thread and re-raised the "(sections 3 to 6)" mislabel alone, declined a second time on the maintainer's call, since the clause isdevelop's own and #1206 holds both halves of it, the widened labels and the excluded section 1, where this finding would fix only the first. The fourth diff pass raised the un-enumerated tie-break as contradicting "authoritative for sections 3 to 6" two sentences earlier, the pair that stood ondevelopand that #1206 files as one paragraph giving two answers, so it is pre-existing too, and the second diff pass had asked for exactly that unscoped form. The pre-existing findings count is 44, #1206 gaining a fourth.🤖 Generated with Claude Code