core: new-repo skill and doctor --profile — a repo standard from a profile - #36
Merged
Merged
Conversation
A profile is the standard a fleet of repos is held to, as one versioned JSON file. It is an argument, never a location: maintainerd ships no profile and names no org. The document that mattered to get right is resolution, because two of its rules are easy to state and easy to implement wrongly. An explicit `null` in a layer overrides the layer beneath it while an absent key inherits — that distinction is the only thing letting one repo be coverage-exempt while every other repo of its language ratchets, so a resolver that treats missing and null alike is wrong in precisely the case somebody wrote an override for. And `requiredChecks` is additive with no way to subtract, because a standard whose required checks can be removed per repo is a suggestion. Two corrections to the shape the design sketched, both because a check was otherwise unimplementable rather than because the shape was ugly: - `defaults.claudeSettings` — the file table requires a settings.json declaring the expected marketplaces and plugins, and nothing in the profile said which. It is optional, and its absence is reported as "not specified" rather than guessed at. - `protection.strictRequiredChecks` — the protection PUT has to send `strict` one way or the other. Default false: with a merge queue, strict is redundant and a good way to make a busy repo unmergeable. And one key documented as deliberately unchecked: `requiredReviews.countsBotApproval` has no GitHub setting behind it. It stays in the profile as the fleet's intent for the review skills, and the settings diff names it rather than passing it silently, because an unchecked key that reads as a pass is how a report loses its meaning. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
Two skills read a profile, so the merge rules live in a script rather than in prose twice. `profile-resolve.sh` validates the shape and prints one repo's effective settings; `settings-diff.sh` compares those against GitHub and prints the exact call that fixes each difference. `settings-diff.sh` reads FILES, never the network. The caller captures the `gh api` responses and hands them over. That is what lets the same diff be produced in a dry run, in a test, and in a report a human reads before pasting anything — and it means the code that computes what should change can never be the thing that changes it. Three behaviours the tests exist to pin, each one a way a wrong standard could look applied: - A read that never happened is "couldn't verify", not a difference. Protection and rulesets need admin on most repos, and a drift report that turns a read-only token into six fabricated diffs is how a weekly drift issue gets muted. - Branch protection is fixed by ONE consolidated PUT carrying the whole desired state. The endpoint replaces the object, so a call sending only the diverging key clears every key it omits — including the required checks. The per-key differences are printed as that call's reasons, not as calls of their own. - The merge queue is a ruleset rule, not a protection key, so its fix is a ruleset POST and its read needs `includes_parents=true` — an org-level parent can be what supplies it. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
Scaffolds every file the profile requires — settings.json, the CI workflow with the coverage-ratchet steps, PR template, CODEOWNERS, dependabot config, review rules — runs `bootstrap` for the repo config, creates the labels, and applies the GitHub settings. Idempotent: a no-op on a conformant repo, and on a partial one it reports what would change and asks, per section, so somebody adopting a repo they don't own can take the files and decline the settings. The skill has a hard seam through the middle. Files are reversible with `git checkout` and can be written anywhere. Branch protection, merge methods and a merge queue are org configuration with the blast radius of a production write, so that half runs only in an interactive session, with the operator's own token, after they have read every call. It refuses — not degrades — when `CI`/`GITHUB_ACTIONS` is set, when `GH_TOKEN` is in the environment (an app token rather than a person), or when the session cannot ask a question at all. A refusal still does the scaffold and still prints the calls; what it will not do is leave a standard half-applied by nobody in particular. Two stops worth naming. It will not write a required check into branch protection without finding a producer for it first: a required check nothing produces blocks every merge in the repo forever, looks identical to a working one, and fails on someone else's PR rather than on this run. And it scaffolds one CI job, not one per required check — `commands` describes exactly one pipeline, so a second job named after a second check would mean inventing what it runs, and a required check whose job does nothing is worse than one that doesn't exist. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
`--profile <path>` adds three checks after the coverage ratchet: the files the profile requires (14), its GitHub settings diffed key by key with the fixing `gh api` call (15), and a producing job or check run for every required check (16). Check 15 is report-only, and that is not a missing `--fix` waiting to be written. The skill already draws its mutation line at creating labels; settings writes sit on the far side of it permanently, and `new-repo` — interactive, human token, every call read first — is where they belong. `--fix` does not change what check 15 does. Check 14 checks existence, not content, for every file but CODEOWNERS. A PR template and a review-rules file are the repo's own prose; a check that diffed their text would report every improvement as drift. CODEOWNERS is the exception because an ownership rule that varies per repo isn't one. And a `files.*` key the profile doesn't carry is reported as not specified rather than as a pass — silence should not read as approval. Check 16 matches on the check-run name, which is per job and not per workflow: the same rule check 13b already uses, and the same mistake it exists to catch. A workflow file called ci.yml produces no check called `ci` unless a job in it is called that. The report block is shaped to be an issue body, because the intended consumer is a weekly drift issue per repo — one issue, titled and labeled the same way each week, updated in place and closed on conformance. Maintainerd ships the check; the repo list and the credentials live with the fleet, so the scheduler does too. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
A new skill and two new scripts reach nobody until the version moves — the install cache is keyed by it. Minor, since `new-repo` and `doctor --profile` are additive. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
Both skills assumed every repo scaffolds and carries `.github/workflows/ci.yml`. A `none`-language repo — tracked but not built here, which is exactly how a profile describes a tooling repo — has an empty `requiredChecks` and no commands, so there is no `ci` job to look for and its absence is not drift. Reporting one would make the first repo anybody adopts fail a check for conforming. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Two review findings, both about the same trap: this script prints a call a human will paste, so anything it gets wrong is applied by hand with full confidence. **The replacement dropped safeguards the profile has no opinion on.** Branch protection has more keys than the seven a profile governs, and the endpoint is a PUT. A body built from the profile alone would switch off required conversation resolution, a locked branch, blocked creations or push restrictions as a side effect of fixing a merge method — silently, and in the direction of less protection. The body now carries those through from the branch as read. Two things the GET cannot be round-tripped into a PUT are warned about rather than dropped quietly: review bypass allowances, whose object shape the PUT will not accept, and app-pinned required checks, which the profile's context-only list would unpin. **A failed read read as an unprotected branch.** GitHub answers "you may not read this" and "this branch has no protection" with the same JSON shape. Only the not-protected message means the branch is open; a permissions error, a 404 or a rate limit means the current state was never established. Those now report couldn't-verify and print no replacement call at all — the alternative is fabricated drift in a weekly issue, and a blind write pasted from it. Seven new assertions cover both, including that the preserved keys actually survive into the printed body. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
…the profile Third review finding, and the general form of the first two: a body whose keys default to the profile's opinion switches off everything the profile is silent about. Requiring zero approvals nulled `required_pull_request_reviews` outright, taking code-owner review and last-push approval with it — those share the object with the approval count and say nothing about it. So the body is now built from the branch as read, with the profile's opinions laid over it. Silence inherits; only an explicit value changes anything. That fixes the reported case and the same bug in `enforce_admins`, `allow_deletions`, `allow_force_pushes`, `required_linear_history` and `strict`, which were all one `//` away from it — and `//` was the trap, since jq treats `false` as empty and would have promoted every disabled setting to the fallback. Also adds `bash -n` over both scripts as the first assertion in the suite. Both embed a long single-quoted jq program where one unescaped apostrophe in a comment ends the quote and turns the rest into shell; the symptom is twenty unrelated assertion failures, and the check costs a millisecond. Learned the hard way, twice, writing this commit. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
… about
Fourth review finding, and the right one: a warning above a call that still loses the
thing is a warning read after the paste. The gate on printing the replacement ignored
the warnings, so an operator following the report would have dropped review bypass
allowances or unpinned app-scoped required checks anyway.
The premise the warnings rested on turned out to be wrong, which is the better fix.
Bypass allowances round-trip exactly like `restrictions` — objects in, logins and slugs
out — so they are a translation, not a guess. And the PUT accepts
`required_status_checks.checks[{context, app_id}]`, not only a bare `contexts` list, so
a pinned check keeps its pin: the profile's context list is written in that form on a
branch that pins, with each observed app_id carried across.
One warning survives, because it is a real widening nothing can avoid: a check the
PROFILE ADDS to a branch whose existing checks are pinned has no pin to carry, so any
app could satisfy it. It names the added checks.
Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
… about Follow-on to the previous commit: doctor's check 15, new-repo's step 9 and the script's own header still described bypass allowances and app pins as unpreservable. They are preserved now, and the only protection warning left is the one nothing can avoid — a check the profile adds to a branch whose existing checks are pinned. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
…ot the response Two shape corrections to the call this prints, both of which would have made the repair fail rather than misapply: - `required_status_checks.contexts` is deprecated but still **required** by the PUT schema, so it is sent whether or not `checks` is. Sending `checks` alone reads as the modern form and is rejected. - `app_id` is an optional integer in the *request*; `null` is the *response* shape. An unpinned check now omits the key instead of sending a null the schema doesn't take. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
…had spliced The "what the replacement preserves" passage had been rewritten twice in place and the second edit landed mid-sentence, leaving the old warned-about-and-dropped text stitched onto the new preserved-and-translated text. It now says one thing: the body is built from the branch as read, silence in the profile inherits, and the values whose GET shape differs from their PUT shape are translated — with the single unavoidable widening named at the end. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
…and the last Fifth finding in this class, and the one that made the pattern obvious: GitHub returns an actor list as user/team/app objects in three places — top-level `restrictions`, `dismissal_restrictions` and `bypass_pull_request_allowances` — and accepts all three as logins and slugs. I had translated two of them and open-coded each, so the third read as a key nobody had thought about, which is exactly what it was. Dropping it broadens who may dismiss a review, every time an unrelated protection key is fixed. One `actors` helper now does the translation, used by all three. And `required_pull_request_reviews` is covered completely: all six of its fields are carried — the two the profile has an opinion on, and the four it does not — which is stated in a comment above the function so the next reader can check the claim rather than infer it from the code. Claude-Session: https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
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
1 participant
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.
Closes #35.
A repo profile is the standard a fleet of repos is held to, as one versioned JSON file. This PR adds the mechanism that applies it and the mechanism that reports drift from it. Maintainerd ships no profile and names no org — the profile is an argument.
What's here
plugins/core/references/profile-schema.md— the contract: shape, resolution, what the profile deliberately does not govern, the versioning rules, and the weekly drift-issue cadence it is built for. Worked example atreferences/example-profile.json.plugins/core/scripts/profile-resolve.sh— validates a profile and resolves one repo's effective settings (defaults+ language + override, key by key).plugins/core/scripts/settings-diff.sh— diffs those against capturedgh apioutput and prints the exact call that fixes each difference.plugins/core/skills/new-repo/SKILL.md— create or--adopta repo against a profile.doctor --profile <path>— checks 14 (files), 15 (GitHub settings), 16 (required-check producers).scripts/test-profile.sh— 77 assertions, wired intovalidate.yml.The decisions worth reviewing
Report-only is a boundary, not a TODO.
doctor --profileprintsgh apicalls and runs none of them, and--fixdoes not change that.new-repo's mutating half refuses outright — not degrades — whenCI/GITHUB_ACTIONSis set, whenGH_TOKENis in the environment (an app token rather than a person), or when the session cannot ask a question. It still scaffolds and still prints the calls; what it won't do is leave a standard half-applied by nobody in particular.Branch protection is fixed by one consolidated PUT. The endpoint replaces the whole object, so a call sending only the diverging key clears every key it omits — including the required checks. Per-key differences are printed as that call's reasons. The helper also warns when the PUT would drop a check the branch requires and the profile doesn't name.
A read that never happened is "couldn't verify", not a difference. Protection and rulesets need admin on most repos, and a drift report that turns a read-only token into six fabricated diffs is how a weekly drift issue gets muted.
A required check with no producer is a stop. It blocks every merge in the repo forever, looks identical to a working one, and fails on somebody else's PR rather than on the run that created it. Matching is on the check-run name, which is per job and not per workflow.
settings-diff.shreads files, never the network — which is what makes the whole GitHub-settings diff testable offline, and means the code computing what should change can never be the thing that changes it.Two corrections to the shape the issue sketched
Both because a check was otherwise unimplementable, not because the shape was ugly:
defaults.claudeSettings(optional) — the file table requires asettings.jsondeclaring the expected marketplaces and plugins, and nothing in the profile said which. Absent, check 14 reports "not specified by the profile" rather than inventing an expected plugin set.protection.strictRequiredChecks(optional, defaultfalse) — the protection PUT must sendstrictone way or the other. False by default: with a merge queue, strict is redundant and a good way to make a busy repo unmergeable.And one key documented as deliberately unchecked:
protection.requiredReviews.countsBotApprovalhas no GitHub setting behind it. It stays in the profile as the fleet's intent for the review skills, and every report names it as unchecked rather than passing it silently.One scaffolded CI job, not one per required check.
commandsdescribes exactly one pipeline, so scaffolding a second job named after a second check would mean inventing what it runs — and a required check whose job does nothing is worse than one that doesn't exist. Every other effective check names a workflow the repo already carries; check 16 is what stops an unproduced name from reaching protection.What review changed
Five rounds, all on
settings-diff.sh, and all one family: a body built from the profile switches off everything the profile is silent about. Branch protection is a PUT that replaces the whole object, and the call this prints is one a human pastes, so every omission is applied by hand with full confidence.The body is now built from the branch as read, with the profile's opinions laid over it. Silence inherits; only an explicit value changes anything. Specifically, over the five rounds:
requiredReviews.count: 0nulled the whole review object, taking code-owner review and last-push approval with it. Generalized://was the trap throughout, since jq treatsfalseas empty and would have promoted every explicitly-disabled setting to its fallback.contextsis deprecated and still required by the request schema, andapp_idis an optional integer there (nullis the response's spelling).dismissal_restrictionsis the third actor list, alongsiderestrictionsand bypass allowances. Oneactorshelper now translates all three, andrequired_pull_request_reviewsis covered completely — all six fields.The last round produced no findings.
Verification
sync-references.sh --check,check-links.py, everyvalidate.ymlstep,test-coverage.sh,test-profile.sh(77/77), andclaude plugin validate .all pass locally.Resolution was also exercised against a real-world profile from outside this repo — seven repos across four language keys — and it validated unchanged and resolved as its design doc describes: additive override checks, a language-level exemption, and a repo-level explicit
nullbeating its language's ratchet.https://claude.ai/code/session_01HE7kFvQacjDaUYSwM1cbbc
The PR appears safe to merge.
Summary
new-repoand extendsdoctorwith profile-based file, settings, and required-check validation.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR P[Versioned repo profile] --> R[profile-resolve.sh] R --> E[Effective repository settings] E --> N[new-repo] E --> D[doctor --profile] N --> F[Scaffold repository files] N --> G[Interactively apply GitHub settings] D --> C[Compare files, settings, and check producers] C --> O[Report drift and print repair commands]