Answer the four spec questions the PhotoCleaner audit raised - #514
Conversation
All four verified against the current tree before changing anything. The carried copilot-instructions section still described the pre-split AGENTS.md, saying most of it is fleet law and two of its sections are repo-specific. After the split GOVERNANCE.md holds the rule sections and the two intent ones, and AGENTS.md carries exactly two verbatim sections and no repo-specific ones. Every repo carrying the section inherited the wrong description, and a reviewer following it looked for byte-locked rule text in the wrong file. CODESTYLE.md said MD033 flags HTML elements while .markdownlint-cli2.jsonc allows details and summary. The config's own comment documents the exception the prose denied. The HISTORY.md mirror rule lived only in spec/readme-structure.md, which is hub-only and carried by nobody, so a repo could not read the rule it was graded against and PhotoCleaner wrote a local copy for want of a destination. It moves into CODESTYLE.md, which every repo carries, and the spec now states only what the audit does with it rather than restating the rule. WORKFLOW.md D2.2 said the gate "is skipped on smoke", which names the validation, and a review read it as the job status and proposed a job-level if:. That would have been a real regression, since github-release has validate-release in its needs and a skipped need skips the dependent. The wording now says the check exits early while the job still reports success, and says why the distinction matters. Reported by the PhotoCleaner agent on #509. Finding 3 landed separately in #511. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Not ready to approve
WORKFLOW.md’s new wording asserts an absolute needs: skip behavior that is contradicted by existing workflows using if: always(), so the documentation should be tightened for technical accuracy.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR updates the fleet’s carried governance/spec documentation to resolve the remaining spec questions raised in #509 (with finding 3 addressed separately in #511), aligning prose with the current tree and audit behavior.
Changes:
- Clarifies
WORKFLOW.mdD2.2 to distinguish “skip validation” vs “skip job” behavior on smoke builds. - Updates
CODESTYLE.mdto match the actual markdownlint MD033 configuration (allowingdetails/summary) and relocates theHISTORY.mdmirror rule into a carried home. - Adjusts
.github/copilot-instructions.mdandspec/readme-structure.mdso the described sources of truth and rule pointers match the post-split governance structure.
File summaries
| File | Description |
|---|---|
WORKFLOW.md |
Clarifies smoke behavior for the D2.2 validation gate to avoid job-level skip semantics. |
spec/readme-structure.md |
Replaces the embedded HISTORY.md mirror rule with a pointer to CODESTYLE.md and states only the audit behavior. |
CODESTYLE.md |
Aligns MD033 prose with .markdownlint-cli2.jsonc and adds the HISTORY.md mirror rule under “Markdown and Spelling”. |
.github/copilot-instructions.md |
Updates carried-content review guidance to reflect governance/router split (GOVERNANCE.md vs AGENTS.md). |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
The D2.2 clarification claimed everything carrying a skipped job in needs: skips with it. That is not true of a dependent that opts out, and this repo carries the counter-example: check-workflow-status has needs: [validate] with if: always(), so it runs and reads needs.validate.result explicitly. The warning still holds where it matters, and now says so precisely. github-release carries validate-release in needs: with if: inputs.github && !inputs.smoke and no always(), so a job-level skip there would couple the release to smoke through a second path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Ready to approve
The changes are documentation-only clarifications that align with existing config/enforcement, with only a minor internal consistency nit noted.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
spec/readme-structure.md:53
spec/readme-structure.mdstates that reference links are grouped under specific headers (e.g.,<!-- Repo -->), but the newly added CODESTYLE pointer is filed under<!-- Internal -->. Using the documented header keeps the file self-consistent and avoids inventing a new category name.
<!-- Internal -->
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
The new CODESTYLE pointer was filed under `<!-- Internal -->`, copied from spec/section-model.md. This file enumerates the group headers two sections above, as Shields, Workflow, Repo and External, so it was inventing a fifth name in the file that states the four. `[codestyle]` is a local repository path, which is what Repo names, so the block now uses it. Found as a suppressed finding in the Copilot round. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The suppressed finding was real, fixed in
I used Worth flagging separately rather than fixing here: That is the ninth low-confidence finding today that turned out to be real, against zero that were noise. |
There was a problem hiding this comment.
🟢 Ready to approve
The changes are prose-only clarifications that match the current tree/configuration and resolve the documented audit findings without introducing behavioral risk.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
… the carried-file corrections to main (#517) Promotes twelve commits to `main`. Three of them change **carried** files, so downstream repos read the stale text until this lands, which is what makes the promotion the delivery step rather than bookkeeping. ## What this delivers to the fleet **Carried-file corrections**, picked up by every repo on its next re-vendor: | PR | File | Correction | | --- | --- | --- | | #514 | `.github/copilot-instructions.md` | Described the pre-split `AGENTS.md`, wrong in both halves. `GOVERNANCE.md` holds the rule sections and the two `intent` ones; `AGENTS.md` carries two verbatim sections and none repo-specific | | #514 | `CODESTYLE.md` | Said MD033 flags HTML elements while the config allows `details` and `summary`; also gains the `HISTORY.md` mirror rule, moved from a hub-only spec file so a repo can read the rule it is graded against | | #514 | `WORKFLOW.md` | D2.2 "skipped on smoke" read as the job rather than the validation, and a review acting on it proposed a change that would have coupled the release to smoke | | #503 | `.editorconfig`, `.gitattributes` | 36 comment findings in the files a new repo copies first | | #501 (earlier) | `repo-config/configure.sh` | Already on `main` | **Spec and requirement changes**: - **#507** makes `OPERATIONS.md` `appliesTo: "*"`, keyed to the need rather than to `workflowModel`. Measured: 3 of 21 repos carry it, so **17 will report a new `LETTER file:` finding**. Intended, and a headed stub satisfies it. - **#512** declares `.github/workflows/merge-bot-pull-request.yml` at `interface` fidelity, closing a graded contract (`WORKFLOW.md` D8.1) that no spec required. 15 of 21 already carry it, so this ratifies a de facto standard; the other **6 report a new `DRIFT`**, and those six have Dependabot pull requests piling up with nothing to merge them. - **#505** stops the audit flagging the hub against itself and drops a premise that was false since this repo went public. - **#511** fixes the README shape across repository visibility. - **#515** puts audit-report authorship with the hub and names the base branch, after a downstream conformance PR targeted `main` and I merged it without checking. - **#513** forward-ports that PR's content, restoring the forward-only invariant. **Backlog and reports**: #504, #506, #510, #516 record the README shape rework, the branch-bootstrap step, the declared-description design, and why the audit is on demand. ## Fleet consequence, stated before it lands Two owed follow-ups this promotion creates or carries: 1. **Re-vendor `repo-config/configure.sh`**, already owed from #501, since it is `verbatim` with `appliesTo: "*"` and every repo holding a copy is byte-mismatched. 2. **The next fleet audit reads noisier**, by 17 `OPERATIONS.md` letters and 6 merge-bot drifts. Both are new requirements rather than repo regressions. ## Verification Run on `develop` at `e4c76bc`: ``` python3 scripts/test_prose_lint.py 144 tests, OK python3 scripts/test_repo_gate.py 23 tests, OK python3 scripts/test_pr_review.py 27 tests, OK python3 spec/audit.py --selftest SELFTEST PASS python3 scripts/repo_gate.py eol 0, sha-pin 0 python3 spec/validate.py OK, 21 cataloged prose_lint --check charset --check dupword --check spelling clean editorconfig-checker (docker, canonical) clean ``` Every one of the twelve was merged green with a Copilot review covering its head SHA. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Answers the four remaining spec questions from #509, raised by the PhotoCleaner agent. Finding 3 landed separately in #511.
Each was verified against the current tree before being changed.
1.
.github/copilot-instructions.mddescribed the pre-splitAGENTS.mdThe carried
Reviewing Carried Fleet Contentsection said most ofAGENTS.mdis fleet law and that two of its sections describe this repository's own directory tree and devcontainer.Verified wrong in both halves against
spec/files.json:AGENTS.mddeclares exactly two sections, bothverbatim, and no repo-specific ones.GOVERNANCE.mdholds the sixteen rule sections and the twointentones,DevcontainerandRepository Layout.It is carried at
intent, so every repo inherits the stale description, and a reviewer following it looks for byte-locked rule text in the wrong file. Now namesGOVERNANCE.mdas the fleet-law file and describesAGENTS.mdas the thin router carrying two byte-locked sections of its own.2.
CODESTYLE.mdcontradicted.markdownlint-cli2.jsoncon MD033CODESTYLE.mdsaid HTML elements are flagged..markdownlint-cli2.jsonc:10sets"MD033": { "allowed_elements": ["details", "summary"] }, and the config's own comment documents the exception the prose denied: "The details and summary elements are allowed for GitHub collapsibles, which have no markdown equivalent."The prose now matches the config, naming the two allowed elements and why.
3. The
HISTORY.mdmirror rule had no carried homeThe rule lived only in
spec/readme-structure.md, which is hub-only and appears in nospec/files.jsonentry, so no repo carries it. A repo could not read the rule it was being graded against, and PhotoCleaner wrote a local copy into its ownCODESTYLE.mdfor want of a destination, which is the duplication the fleet normally avoids.The maintainer's call was to move it where it can be audited, so it becomes item 4 of
CODESTYLE.md"Markdown and Spelling", which every repo carries whole.spec/readme-structure.mdnow states only what the audit does with the rule rather than restating it, so there is one authority and one pointer rather than two copies.4.
WORKFLOW.mdD2.2 "skipped on smoke" was ambiguousD2.2 said the gate "is skipped on smoke", naming the validation. A Copilot review on PhotoCleaner#29 read it as the job status and proposed a job-level
if: !inputs.smoke.That would have been a real regression rather than a style difference:
github-releasecarriesvalidate-releasein itsneeds, and a skipped need skips the dependent, so the release job would have been coupled to smoke through a second path on top of theif:it already carries.The canonical was right and the wording invited the misreading. It now says the check exits early while the job still reports success, and states why the distinction matters.
Verification
spec/readme-structure.mdcarried no links at all, so the new pointer toCODESTYLE.mdis its first. It is written reference-style with a definition block at the bottom, per the rule that same file states, rather than inline.Three of the four are carried files (
.github/copilot-instructions.md,CODESTYLE.md,WORKFLOW.md), so downstream repos pick these up on their next re-vendor.🤖 Generated with Claude Code