Repository navigation
Check Declared Deployment Environments and Fix the Carried-Payload Rule - #1492
Merged
Merged
Conversation
Settles both halves of #1482, both artifacts of the same repo-config/ retirement. GOVERNANCE.md "Hub-Hosted Tooling" listed the ruleset payloads among what a repository carries. No repository carries one: no spec/files.json entry declares them, spec/divergences.json gives all three a retire disposition downstream, and both configure.sh and spec/audit.py diff a repository's live configuration against the hub's. An agent running AUDIT.md against a converged repository could read that clause, find no payloads in the tree, and record a branch-model miss for content the fleet deliberately retired. The clause is deleted rather than restated, since the paragraph's own reason clause already covers a file holding no per-repo content. spec/fidelity-model.md drops the payloads from its intent-fidelity examples, where they had become an example of nothing. Nothing hub-side applied or checked a deployment environment. The retired per-repo configure.sh carried a check_environment and the hub-hosted replacement never had one, so the retirement was a net loss of coverage for the two repositories that use an environment. configure.sh check gains one, driven by a new registry environments field. It asserts that each declared environment exists, that its deployment-branch policy is the declared form, and under a custom policy that the branches it allows are exactly the declared set. aiopurpleair's pypi environment declares the two branches its OIDC publish may mint a token from. Blog's two declare none, recording that its ref gate is a job in the deploy workflow rather than a branch policy, so a policy appearing later reads as a change. The check has no apply counterpart, by decision. An environment's secrets and variables are credentials that exist nowhere in the hub, so an apply would create an environment that cannot publish and report success. An environment the registry declares nothing about is reported rather than asserted, since GitHub creates some on its own. Three surfaces claimed an environment-scoped store cannot be enumerated, including spec/secrets.json's own note. Measured against a live environment holding both, the API returns a secret's name but not its value and a variable's name and value, so each now says no tool here queries that store rather than that none can. CI runs no JSON-schema validation, so spec/validate.py enforces the schema's required-fields and branches-iff-custom rules, with unit tests over each shape and one case asserting the live registry against them. Verified: configure.sh check run against ptr727/aiopurpleair, ptr727/Blog and ptr727/LanguageTags, covering the custom, none and undeclared paths; the missing-environment, wrong-policy and wrong-branch-set failures each reproduced by mutating the registry and reverted; the validator proved by mutation to redden its own tests. prose_lint, repo_gate, spec/validate, build_dist --check, ruff, mypy, the Docker lint set and the full 1180-test suite all clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five findings from the local review pass over the previous commit. The `protected` form is removed from the registry vocabulary rather than documented. GitHub's protected-branches-only deployment policy counts classic branch protection rules, docs/repo-config.md already instructs an operator to remove those in favor of rulesets, and no fleet repository has one: main is unprotected classically on ProjectTemplate, aiopurpleair, Blog and LanguageTags alike. Declaring it would have asserted a gate that does not exist, and check would have printed ok for an environment admitting every ref. The form is still derived from the live state, so an environment set to it fails as the drift it is, and the schema, the validator and docs/repo-config.md each say why it is not offered. A custom entry declaring no branches aborted the whole run on `Cannot iterate over null`, after a pass line had already printed and before the drift summary. spec/validate.py rejects that shape, but configure.sh runs against whatever hub checkout the operator has, so the read is now guarded like every other read in the file and reports a FAIL line instead. STANDUP.md's registry-field enumeration gains `environments`, so a repository stood up with a deployment environment is told to declare one. Four surfaces described the check as asserting a policy when it also asserts the environment exists, which is the more consequential half. And docs/repo-config.md said a repository declaring no environment is not checked, where the live list is read regardless and an unreadable one fails the run. Verified: the abort reproduced against ptr727/aiopurpleair with a branchless custom entry and confirmed to report a FAIL line and reach the drift summary after the guard; classic branch protection absent on all four repositories read; the full gate set and 1181 tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four findings from the local review pass over the previous commit, two of them one root cause. The guard added last commit covered one read of five. The four above it stayed bare, so a malformed entry still aborted the whole run under set -e, mid-check, after the four groups before it had printed pass lines and before cmd_check reached its drift summary. Guarding each read would have grown the same defect one read at a time, so the entry's shape is tested once instead, before any read of it. That test is by element type rather than by container, which closes a defect no guard on a read would have caught: a non-string element renders through jq's string interpolation without erroring, so a branches of [1, 2] was asserted against the live policy set as the branch names "1" and "2". It also stops the previous guard's message mis-diagnosing a branches of "main" as an entry declaring no branches. The script header still carried the claim corrected in docs/repo-config.md last commit, that a repo declaring no environment is not checked, where the live list is read regardless and an unreadable one fails the run. And that document's closing sentence still described a clean run as covering a policy alone. Verified by reproduction against ptr727/aiopurpleair: an entry that is a bare string and an environments that is an object each aborted at exit 5 before this change and now report a FAIL line and reach the summary at exit 1; a branches of [1, 2] was silently asserted as branch names and is now refused; a branches of "main" was mis-diagnosed and is now named correctly; and the three live repos covering the custom, none and undeclared paths still pass unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five findings from the local review pass over the previous commit. The guards added there were correct and two shapes never reached them. `.environments // []` folded a declared falsy value into an empty list upstream of the is-it-a-list test, so an environments of false reported that the repo declares none while spec/validate.py called the same entry invalid. Presence decides now, so a declared value reaches the test that judges it. The undeclared-environment note computed its declared-name list across every entry including one the loop had already refused, so a malformed entry raised there, printing jq's own error, a second failure naming a cause that was not what went wrong, and losing the note for the whole repo. Names come only from entries shaped like one. spec/validate.py read an explicit environments of null as absent, where the sibling guard for description in the same file documents the opposite choice deliberately, and the schema rejects null outright. Presence is the test there too, so a null is declared-but-invalid rather than a repo declaring none. And a missing branchPolicy reported through the value branch as 'None', a Python repr differing from the valid value none by case alone, so absence now reports as absence. One comment claimed the once-per-entry test makes a shape it misses fail loudly. Nothing it misses can, since every read below is what the test guarantees. It now says what the test actually is, narrower than the validator's, and what that costs. Verified by reproduction against ptr727/aiopurpleair: an environments of false reported none before this change and is now refused as not a list; an entry that is a bare string raised inside the undeclared-name computation and now leaves the note intact, correctly listing the environment its refused entry failed to declare. Both validator shapes reproduced through spec/validate.py. The three live repos covering the custom, none and undeclared paths pass unchanged, and the full gate set and 1182 tests are green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One finding from the local review pass over the previous commit, which is a hole in that commit's own fix. Reaching the shape test with a declared falsy value was done by emitting the value verbatim, and that covered false and not null. A declared null prints as the bare word null, byte-identical to the sentinel the same expression emits for a repo the registry does not name once, so the compare below could not tell them apart. A registered repo declaring an invalid null was reported as unregistered, the shape test that exists to catch it never ran, and the whole group was skipped with no failure, leaving the check green on a repo whose declared environments were never read. The same commit had just made spec/validate.py call that shape declared-but-invalid, so one commit's two halves disagreed about one value. No JSON scalar is free to serve as that sentinel, so registration is answered by a read of its own and the declaration by a second, each judged on its own terms. The comment claiming the two stayed distinguishable is replaced by one saying why they cannot be, in a single expression. Verified by reproduction against ptr727/aiopurpleair: null, false, 0 and the empty string each reported the repo as declaring none or as unregistered before this change and now all four reach the guard and report that the declaration is not a list; a name absent from the registry and a duplicated one each still report as they did; and the declared environments still pass unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 55 seconds. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (14)
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 |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new environments schema should be tightened to match spec/validate.py so invalid empty names are rejected at the schema layer.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the fleet configuration contract after repo-config/ retirement work by (1) removing the incorrect claim that downstream repos carry ruleset payloads, and (2) adding a registry-driven configure.sh check assertion for declared GitHub Deployment Environments and their deployment-branch policy.
Changes:
- Extend the registry schema and validator to support an optional
environmentsdeclaration per repo, with unit tests covering the accepted and rejected shapes. - Add
repo-config/configure.sh checkvalidation for declared environments, including deployment-branch policy form and exact allowed branches forcustom. - Update hub docs (STANDUP, AUDIT, OPERATIONS, repo-config docs) to describe the new
environmentsregistry field and the check-only asymmetry.
File summaries
| File | Description |
|---|---|
| STANDUP.md | Documents the new optional environments field in registry enrollment guidance. |
| spec/validate.py | Adds environments shape validation and integrates it into registry validation. |
| spec/secrets.json | Corrects the note to state that tools do not query environment stores, rather than claiming they cannot be enumerated. |
| spec/fidelity-model.md | Removes ruleset payloads from the intent-fidelity examples after retirement. |
| scripts/tests/test_spec_validate.py | Adds unit tests for environments validation and asserts the live registry conforms. |
| reports/canonical-review.json | Updates canonical review metadata for touched canonical units. |
| repo-config/README.md | Notes check now also asserts registry-declared environments and links to docs. |
| repo-config/configure.sh | Implements check_environments() and wires it into cmd_check. |
| registry/repos.schema.json | Adds the environments field schema and a branchPolicy enum. |
| registry/repos.json | Declares environments for aiopurpleair and Blog per the new schema. |
| OPERATIONS.md | Documents that check and apply are not exact inverses due to environment assertions. |
| GOVERNANCE.md | Removes the incorrect "ruleset payloads a repo carries" clause from Hub-Hosted Tooling. |
| docs/repo-config.md | Documents the environments registry field and the check-only contract. |
| AUDIT.md | Updates section 6 guidance to include environment assertions in configure.sh check. |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Both findings from Copilot's first review round on #1492, and both were already open findings from the local pass, reported in the pull request body rather than fixed because they arrived past that pass's edit budget. One note served two registration states. A duplicated registry entry was told that nothing is declared to check and sent to verify by hand whether the repo uses an environment, where the cause is the duplicate and the remedy is to resolve it. The two counts render separately now, each naming its own cause. The schema gave an environment's name no minLength and its branches entries none either, where spec/validate.py refuses both empty and whitespace-only and the shell shape test refuses empty. Every other field keyed name in the file already pins it, so an editor reading the declared $schema blessed a shape both gates refuse. branches itself keeps no minItems, since an empty list under a custom policy is the deliberate declaration that the environment allows nothing. Verified: the zero-entry note reproduced against a registry that does not name the repo; the schema checked with jsonschema under Draft 2020-12, refusing an empty name and an empty branch entry while accepting both valid forms and the empty branches list, with the live registry still validating clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A few small but concrete documentation/contract mismatches remain (docstring accuracy, an ambiguous comment, and a PR description claim that no longer matches the code).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 3
- Review effort level: Lite
Two comment findings from Copilot's second review round on #1492, both about text this change put next to something it now describes wrongly. The secrets note said values are never readable via the API. That was true of the secrets it was written about and this change added an environment group beside it, where a variable's value does read. It now says which of the two the API withholds, and that no tool here asks for the one it does not. The environment docstring said a declared entry carries the two fields configure.sh reads. It reads three under a custom policy, branches among them, which the same docstring goes on to describe. It names them instead of counting them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four findings from the local review pass over the previous commit, which answered a reviewer by writing more where less was owed. The secrets note's joining clause inverted. Fronting `which` after a whether-clause left `an environment variable's value is not` reaching back to `environment-scoped`, the nearest adjectival phrase, so the reading available to a reader was that a variable's value is not environment-scoped, which is exactly what it is. The two facts it joined are each true and neither needed the other, so the note keeps the one it exists for and the header two lines above keeps the other. Its trailing clause restated the header as well, in a second sense of "here", and no gate holds it true, so a later change that reads a variable's value would falsify it silently. The header's own twin of that sentence went unswept by the commit that corrected the note, leaving one surface naming the scopes and one not, which is the divergence the correction was made to remove. The docstring's new third field was restated by the exactly-when clause beside it and understated what the check does: configure.sh reads `branches` under a `none` policy too, to refuse it. The clause carries it in both directions alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are internally consistent across schema, validation, script behavior, tests, and documentation, and no concrete defects were found in the updated logic.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 0 new
- Review effort level: Lite
Owner
Author
|
@coderabbitai review |
|
ptr727
added a commit
that referenced
this pull request
Sep 10, 2026
…g, and the Release-Trigger Retirement (#1489) Promotes thirteen merged pull requests from `develop` to `main`. ## What this promotes - **#1486** State the `host-tools.json` carry rule and stop the empty-overlay warning. `spec/files.json` now requires the declaration unconditionally, and `host_gate.py` no longer warns on an empty one. - **#1484** Fan the validator's pytest leg out over a `python-versions` input, so a caller with more than one interpreter no longer silently loses its other legs to a pinned 3.13, with an entry guard that refuses a value `fromJson` would otherwise admit. - **#1476** Retire the publish-on-merge release trigger, which `spec/files.json` applied to an interface contract whose task has no publish-on-merge path. - **#1479** Ship the release caller snippet and retire the not-yet-released claim. - **#1474** Exclude `node_modules` from markdownlint and Docker lint discovery at any depth. - **#1468** Move the carried-surface cluster off decision, recording the measurements and what stays unmeasured on the issues that hold them. - **#1498** Correct the release-caller snippet's publish-on-merge claim, raised by CodeRabbit against this promotion and fixed on its own branch, since a promotion PR's head is `develop`. - **#1494** Settle the markdown lint exclusion seam as a nested config. - **#1492** Check declared deployment environments and fix the carried-payload rule. - **#1501** Move the session handoff to a chain of labeled issues. Two of its units are verbatim fidelity, `AGENTS.md` "Session Scope" and `GOVERNANCE.md` "Communicating with the User", so every fleet repo picks them up on its next resync. - **#1505** Refuse a bare handoff link rather than reading it as no successor, answering a finding raised against this promotion. - **#1504** Reject a deployment environment name or branch carrying surrounding whitespace, answering two more. - **#1506** Encode an environment name before it becomes a path segment, delete a branch that cannot fire, and dedupe a registry name by case, answering two more and declining one on a reproduction. ## Closing references A `develop`-targeted pull request registers no closing reference, so the issues those pull requests settled close with this promotion. #1470 is the one that needs a word. Two of its seven acceptance items are not met as literally written, and the maintainer's decision to close it here anyway, with the basis for both, is recorded on the issue itself. Closes #1483 Closes #1473 Closes #1487 Closes #1482 Closes #1475 Closes #1470 ## Left open deliberately - **#1499**, **#1500** follow-ups #1501 filed rather than fixed. - **#1503** the validator-looser-than-its-consumer class, filed from findings on this promotion and deliberately not closed by it. Five instances recorded, and the fix is a design decision rather than a check. - **#1497** the Docker delta list never says to remove the `push:` block. - **#1472** the fleet-level pin call, deliberately not pre-empted by #1479. - **#1321**, **#1464** carry #1468's decision record and remain in their own scope. ## Verification `origin/main..origin/develop` merges clean, no end-of-line conflict. Each promoted pull request ran the fleet gate set on its own branch before merging; this promotion adds no content of its own. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01817rckKywjfoaEV5WF9TKs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Settles both halves of #1482, which are two artifacts of the same
repo-config/retirement: a correction, and a scope decision the maintainer took as "restore the check, registry-driven, no apply".The carried-payload rule
GOVERNANCE.md"Hub-Hosted Tooling" listed the ruleset payloads among what a repository carries. No repository carries one. Nospec/files.jsonentry declares them,spec/divergences.jsongives all three aretiredisposition downstream, and bothconfigure.shandspec/audit.pydiff a repository's live configuration against the hub's. An agent runningAUDIT.mdagainst a converged repository could read that clause, find no payloads in the tree, and record abranch-modelmiss for content the fleet deliberately retired.The clause is deleted rather than restated. Two attempts to replace it with a sentence explaining the move each drew fresh review findings, and the paragraph's own reason clause, that a file holding no per-repo content is a copy whose only future is to go stale, already covers a payload.
spec/fidelity-model.mddrops them from its intent-fidelity examples, where they had become an example of nothing. That surface is not named in the issue and was found while checking it.The deployment-environment check
Nothing hub-side applied or checked a deployment environment. The retired per-repo
configure.shcarried acheck_environment; the hub-hosted replacement never had one, so the retirement was a net loss of coverage for the two repositories that use an environment.configure.sh checkgains one, driven by a new registryenvironmentsfield. For each declared environment it asserts that the environment exists, that its deployment-branch policy is the declared form, and undercustomthat the entries it allows are exactly the declared branches. An environment the registry declares nothing about is reported in a note, never asserted, since GitHub creates some without being asked.aiopurpleairpypi,custom,develop+mainBlogproductionandstaging, bothnonenonemakes a policy appearing later read as a change rather than as the gate arrivingThere is no
applycounterpart, by decision. An environment's secrets and variables are credentials that exist nowhere in the hub, so anapplycould create the environment and set its policy and still leave it unable to publish, while reporting success.GitHub's third policy form is not offered. "Protected branches only" counts classic branch protection rules, which
docs/repo-config.mdinstructs an operator to remove in favor of rulesets, and no fleet repository has one:mainis classically unprotected on ProjectTemplate, aiopurpleair, Blog and LanguageTags alike. Declaring it would have asserted a gate that does not exist. The form is still derived from live state, so an environment set to it fails as the drift it is.A false claim on three surfaces
repo-config/configure.sh,docs/repo-config.mdandspec/secrets.json's own note each said an environment-scoped store cannot be enumerated. Measured against a live environment holding both, the API returns a secret's name but not its value, and a variable's name and value. Each now says no tool here queries that store rather than that none can.Verification
spec/validate.pyenforces the schema's required-fields and branches-iff-custom rules by hand, since CI runs no JSON-schema validation, with unit tests over each shape and one case asserting the live registry against them. The validator was proved by mutation to redden its own tests.configure.sh checkwas run live againstptr727/aiopurpleair,ptr727/Blogandptr727/LanguageTags, covering thecustom,noneand undeclared paths. Every failure path was reproduced by mutating the registry and reverted: a missing environment, a wrong policy form, a wrong branch set, an entry that is a bare string, a non-listenvironments, acustomentry with nobranches, abranchesof"main", abranchesof[1, 2], andnull,false,0and""as the declared value. Both registration cases, a name absent from the registry and a duplicated one, were reproduced too.Full gate set green:
prose_lint,repo_gate,spec/validate,canonical_review check,build_dist --check, ruff check and format, mypy, the Docker lint set, and 1182 tests.Review
Six
local-strict-reviewpasses ran against this branch, two over each carried canonical unit it changes and four over the diff. They raised 30 introduced findings, every one of which is fixed here, and the last four commits are each one round answering the previous pass. Notable among them: acustomentry with nobranchesaborted the whole run underset -emid-check; abranchesof[1, 2]was silently asserted against the live set as the branch names "1" and "2"; and a declarednullcollided with a sentinel value, reporting a registered repository as unregistered and skipping the group with no failure.Both findings this description originally listed as open are now fixed, in
8dfd668, after Copilot's first round raised the same two independently. A duplicated registry entry was told "nothing is declared to check. Verify by hand whether this repo uses one" when the cause is the duplicate; the two counts render separately now. Andregistry/repos.schema.jsongave an environment'snameand itsbranchesentries nominLength: 1, where every other field keyednamein that file has one; both now carry it, verified withjsonschemaunder Draft 2020-12.The 21 pre-existing findings those passes surfaced in
GOVERNANCE.md"Hub-Hosted Tooling" andAUDIT.mdsection 6 are filed separately and are not touched here.The hub's own live
pypienvironment stays undeclared, deliberately: the hub hosts the generic pypi, nuget and docker machinery without publishing through any of it, so that environment gates nothing and declaring a policy for it would assert a gate that matters when it does not.Settles #1482. The closing keyword belongs on the
develop->mainpromotion pull request, since a closing reference on adevelop-targeted one never registers.🤖 Generated with Claude Code