diff --git a/.editorconfig-checker.json b/.editorconfig-checker.json index 22add8b..a3478e9 100644 --- a/.editorconfig-checker.json +++ b/.editorconfig-checker.json @@ -1,4 +1,11 @@ { + "Exclude": [ + "(^|/)__pycache__/", + "(^|/)\\.mypy_cache/", + "(^|/)\\.pytest_cache/", + "(^|/)\\.ruff_cache/", + "(^|/)\\.venv/" + ], "Disable": { "Charset": true, "Indentation": true, diff --git a/.github/skills/add-host-tool/SKILL.md b/.github/skills/add-host-tool/SKILL.md new file mode 100644 index 0000000..22b10df --- /dev/null +++ b/.github/skills/add-host-tool/SKILL.md @@ -0,0 +1,46 @@ +--- +name: add-host-tool +description: >- + Adds or changes a managed host tool across the ptr727/ProjectTemplate fleet contract, Linux and + Windows installers, platform documentation, and tests. Use this whenever adding, removing, + renaming, or changing the source, probe, version floor, install, report, upgrade, or dry-run + behavior of a tool in host-setup or spec/host-tools.json. Triggers even when the request names + only one platform, because a required fleet tool needs an executable remedy everywhere it + applies and native verification must stay on the platform being tested. +--- + +# Add Host Tool + +## Establish the Contract + +1. Read the issue and all follow-up comments before choosing a source or package identifier. +2. Add the tool to `spec/host-tools.json` in name order. +3. Use the executable's real version banner for the probe and pattern. +4. Set a floor only when it is measured or anchored to every supported distribution. +5. Provide `source` and executable `remedy` entries for every applicable platform. + +## Implement Each Platform + +- Keep the existing named-tool interface and default selection behavior. +- Prefer the distribution package when it meets the floor. +- Use the platform's established package manager and official package identifier. +- Keep install and upgrade idempotent. +- Before an apt-managed install, detect and remove an unowned downloaded copy that shadows it. +- Before a downloaded install, detect and remove a conflicting package-managed copy. +- Preserve report, list, explicit selection, install, upgrade, reinstall, and dry-run behavior. +- Do not test a Windows mutation on Linux or a Linux mutation on Windows. + +When a platform is unavailable, verify its registry and tests without claiming a native install. Hand off the exact native commands and expected observations to the operator. + +## Update the Complete Surface + +Update the platform installers, `spec/host-tools.json`, `docs/host-setup.md`, and the applicable platform READMEs. Update installer and host-gate tests for selection, reporting, installation, upgrade, and dry-run behavior. Sweep prose that describes tool sources or the managed set. + +## Verify + +1. Run the spec validator and the focused installer and host-gate tests. +2. Run the repository's formatting, lint, type, and test gates required by the changed files. +3. On the current native platform, exercise list and report first. +4. Exercise install and upgrade dry runs. +5. Apply the install, repeat it to prove idempotence, and run the host gate. +6. Record untested platforms explicitly and leave cross-platform verification open. diff --git a/.github/skills/agent-conduct/SKILL.md b/.github/skills/agent-conduct/SKILL.md new file mode 100644 index 0000000..060267c --- /dev/null +++ b/.github/skills/agent-conduct/SKILL.md @@ -0,0 +1,47 @@ +--- +name: agent-conduct +description: >- + Surfaces the ptr727/ProjectTemplate fleet's conduct rules at the three decision moments they are violated: about to claim work is done, verified, green, or fixed, about to proceed on an assumption the user could cheaply confirm, and a failure or review finding just surfaced a durable lesson. Use this whenever about to report success or completion of any task, whenever about to pick a default, guess an intent, or resolve an ambiguity without asking, whenever work is blocked on a decision or authorization only the user can give, and whenever an incident, a wrong answer, or a repeated correction just taught something a future session must honor. Deliberately narrow: the carried AGENTS.md sections are the always-on layer, and this skill fires at the moments rather than duplicating them, so do not load it as general background. Where a sibling skill owns the moment, it wins: git-commit-conventions for committing, pr-review-conduct for review and merge claims, comment-and-doc-style for prose. The GOVERNANCE.md sections this skill summarizes keep the full rules. +--- + +# Agent Conduct + +## Why This Exists + +The fleet's conduct rules (verification before claiming done, asking instead of assuming, recording lessons) lived only in doc sections nothing surfaced at the moment of violation, so they were honored by whoever happened to have read them recently. This skill is the decision-moment surface. The full rules stay in `GOVERNANCE.md` ("Verification Discipline", "Communicating with the User", "Durable Knowledge and Self-Improvement"), which keeps authority, and in the carried `AGENTS.md` "Context and Delegation Discipline" section, which is the always-on layer. + +## Before Claiming Done + +Read `GOVERNANCE.md` "Verification Discipline" before reporting success on anything non-trivial. Its unifying property: every failure it lists is green. The checks that bind here: + +- **A green check is not evidence the work happened.** A skipped job and a passing job are indistinguishable in an aggregated required check, so confirm from the log that the job ran and produced what it promises. +- **Locate every check the change owes before running any**, from what the repository declares (`OPERATIONS.md` "Local Verification" beside the workflows), not from what the pipeline happens to run, since part of a contract is routinely unreachable from a runner and green is then the precise signal it was skipped. +- **Run the repo's whole lint gate before every push**, not the parts that look relevant, because the tool most likely to catch a change is often the one it seems least about. +- **A launched process is not a result.** Report the output the wait produced, and where it produced none, that absence is the report. Never name an external cause the record does not carry. +- **A local clone is not the branch it names.** Fetch immediately before reading, or read the live ref, and name the ref and commit in any finding a local read produced. +- **A checkout this session did not create is not ground truth.** One found already sitting on disk may belong to another concurrent session, sit on a stale fetch or an unexpected branch, or hold unreviewed uncommitted edits. Clone fresh or read the live API instead of trusting `git status`/`git remote -v` run against a pre-existing checkout. +- **A "does not exist" claim names the branch it was checked against.** A worktree's default branch is not necessarily the one the content lives on: in-flight content on a `release`-model repo lands on `develop` before `main`, per `GOVERNANCE.md` "Branching Model," so check that branch before reporting anything absent repo-wide. +- **A `raw.githubusercontent.com` 404 does not distinguish a private repository from a missing file.** Where visibility is not confirmed public, read content via `gh api "repos///contents/?ref="`, capturing the result before decoding it (`content=$(gh api ... --jq '.content') && printf '%s' "$content" | base64 -d`) rather than piping straight into `base64 -d`, whose own exit status is all a direct pipe reports, letting a failed fetch decode as an empty success. Never `2>&1` either form, which corrupts the decode with the error text instead of the payload. Verify the ref resolves before reading either failure as proof the content itself does not exist. +- **A test asserts the mechanism it names, and a gate has to be watched failing.** A case that passes for an incidental reason is worse than no case, because it is later cited as evidence. +- **Platform-specific code is verified only on the platform it runs on.** Reasoning about PowerShell, macOS, or WSL-specific behavior from a different host is not verification, however closely it matches an already-tested equivalent elsewhere. State an untested structural match as exactly that, never in the words used for a tested fact, and when no agent in the loop has access to the target platform, say so and defer or ship it labeled unverified. +- **PR-bound work runs `local-strict-review` before the claim.** Claiming a unit of work done, verified, green, or fixed for work that will become, or already is, a pull request means running `local-strict-review` against the branch's diff first, before a PR-hosted reviewer finds the same gap. + +Claims about a pull request being reviewed, clean, or mergeable are owned by the `pr-review-conduct` skill, and claims that a commit landed by `git-commit-conventions`. + +## Before Assuming + +- **Ask when the user can cheaply confirm.** An assumption that saves one question and is wrong costs the rework plus the trust, so a genuine ambiguity in intent, scope, or authorization is raised, not resolved by picking the likelier reading. Rules that already answer the question (the committed instruction set) are not ambiguity, so read them first rather than asking what they state. +- **Raise blocked work as a direct interactive prompt** at the point the work stops, per `GOVERNANCE.md` "Communicating with the User": the blocked item is the message, the options offered are the actions themselves, and a handoff buried in a summary paragraph is a handoff that did not happen. Numbered lists are the fallback where no prompt mechanism exists. +- **References are clickable where they are read**: a pull request, issue, or commit on a Markdown surface is a Markdown link, and on a surface that renders neither, a bare `#123` with the link in the message before the prompt. +- **Capability is not permission.** A token's reach, a tool that happens to work, or a similar grant in a past session authorizes nothing, and the irreversible step (merge, publish, release, delete) stays the maintainer's. + +## When a Failure Surfaces a Lesson + +- **Durable knowledge lands in the committed docs, not in agent memory**, as part of the change that surfaced it, per `GOVERNANCE.md` "Durable Knowledge and Self-Improvement". Memory does not survive a new session or machine, so it holds only environment nuance and in-flight state. +- **Where the governing doc is carried from the hub, file the finding against `ptr727/ProjectTemplate`** rather than only patching it locally. A local fix leaves every sibling repo with the same trap. Search open and closed issues first, then update the matching issue or file a new one. +- **A review flags an instance, so fix the class**: sweep for the siblings before replying, because reviewers sample rather than enumerate. +- **A rule that keeps needing restating** is usually a stale or missing skills install, so run `python3 scripts/skills_install.py --report` from a hub checkout (the `fleet-conformance-check` skill) before concluding the rule does not exist. + +## Delegation, in One Paragraph + +The always-on rules live in `AGENTS.md` "Context and Delegation Discipline" and are not restated here. The two that intersect conduct: brief a subagent so it never needs a governance file, since anything it must honor has to be in its prompt, and never tier down the seat holding the judgment, because governance wording and the decision to decline a review finding are fleet-wide and durable when wrong. diff --git a/.github/skills/audit-a-repo/SKILL.md b/.github/skills/audit-a-repo/SKILL.md new file mode 100644 index 0000000..5cd0c13 --- /dev/null +++ b/.github/skills/audit-a-repo/SKILL.md @@ -0,0 +1,37 @@ +--- +name: audit-a-repo +description: >- + Drives AUDIT.md's read-only measurement of a named ptr727 fleet repo against the fleet ground truth, ending in a committed report, never an edit to the repo being measured. Use this whenever asked to audit, measure, or verify conformance of a named repo, to judge a conformance claim someone else made, or to decide whether an onboarding is actually complete. Run from a hub checkout of ptr727/ProjectTemplate against the named target. Triggers even when the repo believes it is conformant, because conformance asserted without a committed report is conformance nobody can check, and that is the case most often skipped. This completes the procedure triangle: standup-a-repo creates a repo, resync-a-repo applies findings to one already stood up, and this skill measures, while fleet-conformance-check is the in-repo self-check with no named target and no standing hub checkout. AUDIT.md keeps authority over the procedure, this skill is the summary that routes into it. +--- + +# Audit a Repo + +## Why This Exists + +The audit is the fleet's measurement procedure, and the two failure shapes it guards against are both silent: a repo judged conformant with no committed evidence, and an audit that quietly edits what it was supposed to measure. `AUDIT.md` in the hub is the procedure and keeps authority. This skill carries the rules that get skipped in practice and says which section owns each step. + +## Before Measuring Anything + +- **Route first.** A repo with no carried instruction set, or a partial one, has a baseline that never arrived rather than drift to report, so it goes to `STANDUP.md` sections 1A and 2 first (`AUDIT.md` section 0). Auditing it anyway produces a report that is all absences and reads as catastrophe. +- **Verify the host.** Run `python3 scripts/host_gate.py --repo ` from the hub checkout before any hub tool, and pass `--repo`, since a bare run skips the target's own `host-tools.json` overlay. A stale tool answers `--version`, looks healthy, and produces a wrong answer. +- **Read `main` as ground truth**, for both workflow models, and read `develop` only to detect divergence (`AUDIT.md` section 1). An `operational` repo's `develop` is mid-flight by design, so conformance work sitting there is un-promoted work, not a defect, and it counts when it reaches `main`. Use `spec/audit.py --branch ` to preview in-flight work, which stamps the override so the finding cannot be mistaken for one against ground truth. + +## Measuring + +- **Resolve the repo's types from `registry/repos.json`** and classify a `classificationPending` entry from the tree (`AUDIT.md` section 2). The applicability gate is `WORKFLOW.md` section 1: a check governing an absent construct is N/A, excluded from the verdict, and never a defect (`AUDIT.md` section 3). +- **Know what the runner does and does not prove.** `spec/audit.py` mechanizes the deterministic subset only: settings, rulesets, secret names, file and section presence, verbatim hashing, interface wiring, Dependabot coverage, branch facts. It evaluates no check under a type in `spec/project-types.json`, so every per-type check is judged by hand, and a clean run is no evidence for them (`AUDIT.md` section 4). Silence from a tool that was never looking reads exactly like a pass. +- **Judge letter and intent per check** and keep the vocabulary: letter miss with intent satisfied is a drift finding, both missing is a defect, and operational is binary over the applicable set (`AUDIT.md` sections 4 and 7). Do not invent a parallel scheme. +- **Assert the Actions implement `WORKFLOW.md`** by outcome, not by matching catalog snippets byte for byte: the 5A static audit with a `file:line` citation per applicable guarantee, then the 5B trace scenarios (`AUDIT.md` section 5). The `workflow-ci-contract` skill summarizes that contract. +- **Check live settings, rulesets, and secrets from a hub checkout at `main`** with `AUDIT.md` section 6. Run `repo-config/configure.sh check` with the target repository and model for settings and rulesets, and `spec/audit.py [RepoName]` for secrets, rather than constructing a local comparison. The hub payloads are the only repository-configuration source. + +## Reporting + +- **Write `reports//audit.md` from `reports/_template.md`**, findings ranked most severe first, each with the `file:line` it was judged against, and quote the run stamp, since findings are a point-in-time snapshot (`AUDIT.md` section 8). +- **The hub authors the report.** A downstream repo never opens a hub pull request to write its own, which would be self-certification. Downstream context goes into issues filed against the hub instead. +- **Generate a convergence issue, never compose one**: `spec/audit.py --issue ` emits it from live findings. An agent picking such an issue up re-runs the audit first and acts on the live result, not the pasted findings. +- **Reconcile registry `driftNotes` in the same pass**: a resolved deviation's note is deleted, not left describing finished work, and a note naming a check id is retired by a person, not by a run (`AUDIT.md` section 8). +- **Stale-versus-modified classification needs a full hub clone with git history.** Without one, compare against the current hub canonical on `main`, which decides current-match only. + +## After the Report + +Measuring and fixing are separate phases. Converging is `AUDIT.md` section 10: fixes ship as pull requests on the target repo, one focused pull request per drift class, the Copilot loop driven to green per the `pr-review-conduct` skill, and the maintainer merges. For a repo already stood up, `RESYNC.md` sequences the findings, since order matters (a deletion lands before the re-vendor that would refresh it). Systemic drift shared by many repos is fixed in the hub spec, not hand-patched per repo, and spec questions are escalated rather than resolved silently (`AUDIT.md` section 9). diff --git a/.github/skills/carried-instruction-file-guard/SKILL.md b/.github/skills/carried-instruction-file-guard/SKILL.md new file mode 100644 index 0000000..47f8803 --- /dev/null +++ b/.github/skills/carried-instruction-file-guard/SKILL.md @@ -0,0 +1,31 @@ +--- +name: carried-instruction-file-guard +description: >- + Stops a blind overwrite of a downstream repo's AGENTS.md, GOVERNANCE.md, CODESTYLE.md, or WORKFLOW.md when resyncing or updating it to match the ptr727/ProjectTemplate hub template. Use this whenever about to edit, replace, re-vendor, or sync-to-match-the-hub any of those four files in a repository that is not ProjectTemplate itself, or whenever asked to bring a repo's instruction set up to date, run a conformance sweep, or fix drift against the hub. Triggers even when the request sounds routine, such as copying the hub's AGENTS.md over or resyncing a repo's docs, because that phrasing is exactly how a real incident happened, where a downstream repo's local rules were silently deleted by a full-file overwrite. Do not skip this just because the task looks mechanical. +--- + +# Carried Instruction File Guard + +## Why this exists + +A downstream repo's `AGENTS.md`/`GOVERNANCE.md`/`CODESTYLE.md`/`WORKFLOW.md` can hold two different kinds of content mixed in one file: sections that are stale copies of the hub's fleet-wide rules, and local rules the repo wrote for a fault the fleet has never seen elsewhere. Re-vendoring the hub's canonical version over the whole file deletes the second kind silently, because nothing about the diff looks wrong. This has actually happened: a resync replaced a repo's `AGENTS.md` wholesale with the hub's, and the repo's own local additions were gone with no error, no warning, and no review comment calling it out. + +The fix is not "be careful." Being careful is what failed the first time. The fix is a mechanical check you run before any overwrite touches one of these four files, every time, regardless of how routine the request sounds. + +## Before you touch any of these four files + +1. **Check whether the file's content is declared `verbatim` or `intent`.** The hub's `spec/section-model.md` (fetch it from a hub checkout, `github.com/ptr727/ProjectTemplate`, if you don't have one) names, section by section, which parts of `AGENTS.md` and `GOVERNANCE.md` are universal fleet law (safe to byte-match against the hub) and which describe the repo itself (never safe to overwrite from another repo). `CODESTYLE.md` and `WORKFLOW.md` are carried whole at `intent` fidelity, judged by meaning, not hashed. +2. **If any part of the file is `intent`, or if the file predates a clean split into hub-governed sections, do not diff-and-replace. Probe instead.** For each rule or paragraph in the current file that is not obviously boilerplate: + - Pick the phrase in it that is most peculiar to this repo, not generic governance vocabulary. A rule about "always sign commits" is generic. A rule about "this repo's Docker image pins Alpine 3.19 because 3.20 broke the s6 supervisor" is peculiar. + - Grep the hub's canonical copy of the same file for that peculiar phrase. + - **Absent from the hub canonical means it is a local addition.** It is never dropped because it looks similar to something else, and never dropped because a merge or overwrite would be simpler without it. +3. **A local addition found by the probe gets a destination, not a deletion.** Either it names a rule that should apply fleet-wide (flag it for the maintainer to promote into the hub), or it is genuinely specific to this repo and moves to the repo's own topical doc before the carried file is touched: `CODESTYLE.md` for a language/formatting convention, `ARCHITECTURE.md` for a design decision, `OPERATIONS.md` for a runbook or operational note, `TODO.md` for backlog. Move it, confirm it is not lost, and only then proceed with the carry. +4. **Do not trust a similarity or word-overlap check for step 2.** A repo-specific rule written in ordinary governance language reads as a reworded duplicate of an unrelated hub rule to that kind of check, and it will confidently tell you the local content is redundant when it is not. Exact phrase presence or absence is the only check that has held up. + +## What is actually safe to overwrite without this procedure + +A section `spec/section-model.md` names as `verbatim`, in a file that is already cleanly split (the file carries only that declared section, nothing else mixed in), can be re-vendored directly: byte-matching it against the hub canonical is the point of `verbatim` fidelity, and the audit already checks it that way. The guard above is for everything else: `intent`-fidelity content, a file that has not been split yet, or any file you are not certain is clean. + +## If you are not sure which case you are in + +Stop and say so, rather than guessing. Naming the uncertainty costs one sentence. Silently overwriting the wrong thing costs someone's local rules with no way to notice until much later. diff --git a/.github/skills/code-review/SKILL.md b/.github/skills/code-review/SKILL.md new file mode 100644 index 0000000..af77518 --- /dev/null +++ b/.github/skills/code-review/SKILL.md @@ -0,0 +1,71 @@ +--- +name: code-review +description: >- + Reviews a pull request or change set against the repository's contracts, with explicit diff + coverage and no suppressed findings. Use this whenever asked to review code, a pull request, + a patch, or a proposed change, and whenever GitHub Copilot performs code review. Triggers even + when the diff is documentation-only or workflow-only, because the review must load the + applicable general, language, documentation, and workflow skills before judging the change. +--- + +# Code Review + +## Establish the Contract + +1. Read the root `AGENTS.md` and the sections it routes to for the changed paths. +2. Read the complete diff and enumerate every changed file before forming findings. +3. Load every applicable sibling skill from the current skill distribution: + - `comment-and-doc-style` for Markdown, prose, comments, commit messages, and PR titles. + - `dotnet-codestyle` for C# and .NET changes. + - `python-codestyle` for Python changes. + - `shell-codestyle` for shell changes. + - `workflow-ci-contract` for GitHub Actions and CI/CD changes. +4. Treat a missing executable on `PATH` as no evidence that its check is unavailable. Read the + repository's documented local invocation before reporting a check as skipped. + +Do not substitute a familiar convention for the repository's written contract. Report a +conflict between instructions instead of silently choosing one. + +## Review the Change + +Review for correctness, regressions, security, compatibility, error handling, concurrency, +resource lifetime, tests, and contract drift. Follow data and control flow beyond the edited +lines when the behavior depends on unchanged callers or consumers. + +For each candidate finding: + +1. Verify it against the current head tree, not an unfetched checkout or the base branch. +2. Identify the concrete failing behavior and the conditions that reach it. +3. Confirm that the repository does not already prevent it elsewhere. +4. Prefer one root-cause finding over several symptoms of the same defect. +5. Omit pure preferences that no repository rule or user-visible risk supports. + +Review carried fleet content by intent and fidelity. A byte-locked reference to a path that one +downstream repository does not carry is not a broken link. A substantive defect in canonical +content remains a finding, with the fix located at its canonical source. + +## Publish Every Finding + +Never suppress or hide a finding because confidence is low. Investigate until it is supported +or discard it. Publish every supported finding as an inline review comment when a changed line +can anchor it. Use the review body only when no valid inline anchor exists. + +Each finding states: + +- A concise imperative title with a severity. +- The file and smallest useful line range. +- The behavior that fails and the input or state that triggers it. +- Why the change causes the failure. +- A bounded direction for the fix when one is known. + +Do not report a clean review until every changed file has been read. End the review body with +exactly one ASCII marker, replacing the numbers with measured counts: + +```text + +``` + +`reviewed` is the number of changed files actually reviewed. `changed` is the total number of +changed files. `findings` is the number of published findings, including body-only findings. +Never emit `reviewed=changed` as a placeholder. If full coverage is impossible, emit the actual +counts and explain the limitation in the review body. diff --git a/.github/skills/comment-and-doc-style/SKILL.md b/.github/skills/comment-and-doc-style/SKILL.md new file mode 100644 index 0000000..af574e9 --- /dev/null +++ b/.github/skills/comment-and-doc-style/SKILL.md @@ -0,0 +1,251 @@ +--- +name: comment-and-doc-style +description: >- + Governs prose, comment, Markdown, character-set, line-ending, and PR-title/commit-message + conventions for every ptr727/ProjectTemplate fleet repo. Use this whenever writing or editing a + code comment, workflow comment, Markdown doc, commit message, or PR title, whenever choosing + which characters to type in agent-authored text, whenever the file being edited is CRLF, and + whenever naming a tool in prose or docs. Triggers even when the task looks purely mechanical, + such as "just fix a typo" or "add a one-line comment", because the fleet's ASCII character-set + tiers, no-semicolon rule, comment-growth discipline, and CRLF-preservation rule are each easy to + violate without noticing: an em dash slipped into a sentence, a comment that grew by one more + clause, or a text-mode edit that silently flattens a CRLF file to LF. Also triggers when + authoring a new Markdown file (reference-style links, Table of Contents, present tense), when a + carried instruction file (AGENTS.md, GOVERNANCE.md, CODESTYLE.md, WORKFLOW.md, + .github/copilot-instructions.md) is being edited (no coordination references to the template or + a sibling repo), and when writing a PR title or commit message (imperative subject, no vague + titles, no unsolicited Co-Authored-By, no release-bump magnitude). +--- + +# Comment and Doc Style + +## Why this exists + +These are the fleet's mechanical prose rules, kept in one place instead of re-derived per repo or +per session: how to write a comment, which characters an agent may type, how a Markdown file is +structured, how a carried instruction file may reference the hub, and how a PR title or commit +message reads. None of these are matters of taste. Each is checked, by `prose_lint.py`, +`editorconfig-checker`, `markdownlint`, `cspell`, or a human reviewer, and each has been the exact +subject of a real review finding. + +## Naming tools in prose + +Use each tool's official casing in task labels, docs, and prose: `.NET` (not `.Net`), +`CSharpier`, `ruff`, `pyright`, `uv`. Do not invent personal variants. + +## Markdown files: linting and spelling + +- **Markdown lints clean, repo-wide.** Every `.md` file is error and warning free via + `markdownlint-cli2` against the shared `.markdownlint-cli2.jsonc`. A rule it deliberately + disables (for example `MD013` line length) stays disabled, do not "fix" it. `MD033` inline HTML + stays enabled: HTML comments, and `details`/`summary` (no Markdown equivalent for a + collapsible), are allowed, everything else with a native Markdown equivalent uses the Markdown. +- **Spelling is US English**, checked by CSpell against the shared `cspell.json` + (`"language": "en-US"`, so a British spelling is flagged). Add a project term to `cspell.json`'s + `words` list, never to a `.code-workspace`'s own `cspell.words` block. +- **CI's spelling gate covers `README.md` and `HISTORY.md` only**, deliberately not every `.md` + file, so a new topical doc is not spell-gated in CI (the editor extension still flags it live). + A repo may widen its own CI list, README plus HISTORY is the default. A repo shipping no + `HISTORY.md` drops it from the CI workflow, the `Lint: Spelling` task, and the GOVERNANCE.md + cspell line together, all three or none. +- **`HISTORY.md` mirrors the README's opening**: the same `# `, the same tagline verbatim + (the first line after the README's H1), then its own `## Release History`. It never repeats a + paragraph below the README's tagline. +- **"Markdown" is a proper noun in prose** (a Markdown file, a Markdown-only repo), lowercase only + for what a machine reads: a tool or package name (`markdownlint`), a settings key, a heading + anchor, a file extension. + +## Docker lint authorization + +A restricted executor treats Docker socket access, image fetching, and repository exposure as +separate permissions. Repository exposure needs explicit maintainer approval even when the mount +is read-only. Use the hub's `scripts/docker_lint.py` wrapper for the standard lint shape. It +discovers targets, pulls images in a separate phase, resolves each digest, and announces the +boundary before repository mounts begin. Each Docker command has a timeout and visible result. +Lint containers disable networking and mount the checkout read-only. Persist approval only when +the executor constrains that whole shape. Never allow an unconstrained `docker run` prefix. +PSScriptAnalyzer downloads its pinned module in a separate container that has network access and +no repository mount. `GOVERNANCE.md` "Running the Linters Locally (Known-Working Invocations)" +owns the exact invocation and full authorization model. + +Agent-specific authorization stays in provider-labeled bullets so one agent's configuration does +not read as a shared requirement: + +- **Codex:** rules cannot safely cover changing worktree paths and digests. Smart Approvals can + prompt per task. No-prompt operation is supported only inside an external sandbox because it + removes command-wide protection. + +## Markdown formatting + +- **Reference-style links everywhere**, except the four files read one section at a time rather + than end to end: `AGENTS.md`, `GOVERNANCE.md`, `OPERATIONS.md`, `.github/copilot-instructions.md`. + Those keep inline links so a target resolves where it is read. Every other Markdown file defines + every URI at the bottom, grouped by type under an HTML-comment header, each group alphabetized + by reference name rather than by the full definition line (a name that is a prefix of another + sorts first, `[governance]` above `[governance-branching-model]`). A URL inside a fenced code + block stays inline. See `references/markdown-links.md` for the full grouping and naming + convention. +- **Table of Contents**: generated by the Markdown All in One extension on save, never + hand-authored or hand-edited. Exclude a heading with an inline `<!-- omit from toc -->` marker. +- **One logical paragraph per line**, no hard-wrap line-length limit. For an intentional line + break within a block (stacked badges, status lines), end the line with a trailing backslash + rather than trailing whitespace. +- **Headings use the PR-title casing rule** below. +- **Write in the present tense.** State what *is*, never a change from a prior state ("X does Y", + not "X now does Y" or "X no longer does Z"). This applies to docs and code/workflow comments + alike. Before/after framing belongs in changelogs, commit messages, and PR descriptions, where + the prior state is the point. +- **When a behavior changes, grep for prose asserting the old one.** Comments, diagram labels, + workflow-input descriptions, and audit statements elsewhere may still describe the prior + behavior, and each was accurate when written. No linter catches a claim that is merely untrue, + so this sweep is the only mechanism that will. + +## Sentence structure + +The structural half of ASD-STE100 is the adopted house style for agent-authored prose, and the +controlled dictionary is deliberately not adopted: vocabulary stays unrestricted, structure is +restricted. Each structural rule a pattern can reach lands as a `prose_lint.py` check +incrementally, and this section names each check as it ships. + +- **Short sentences: at most 25 words in one sentence**, ASD-STE100's descriptive cap, checked by + the `sentence-length` rule in `prose_lint.py`. The check is opt-in like `sentence-split`, + because the existing corpus predates the cap and a default gate would fail whole files nobody + is editing. Write new prose under the cap, and scope a run to a change with + `--check sentence-length --diff <base>`. +- **One instruction per sentence.** A procedure step states one action, and a second action is a + second step. No pattern reaches this, so it is authoring discipline with no check. +- **Active voice, imperative mood for procedure steps.** Write "run the gate", never "the gate + should be run". Also authoring discipline, since a reliable passive-voice pattern does not + exist. + +## Comments + +Applies to code and workflow (`#`) comments alike. + +- Comment only when the code does not explain itself, or the logic is genuinely complex. + Self-evident code needs no comment. +- State only the non-obvious *why*, for the human reading *this* project's code now. No + cross-project references, no historic or design narrative, no rule citations. Governance lives + in the fleet's own instruction set, not echoed inline. +- **Keep it short**: one line is the default. A second line is earned only by a constraint the + code cannot otherwise carry. +- **Structured, not prose**: one sentence per line, never wrapped across lines, never a + multi-sentence run-on. A comment that genuinely needs several sentences is several lines, each + one sentence. +- A comment line opening prose starts with a capital. A trailing label, or the version pin an + action-pinning rule requires, does not. +- Mark a sub-topic with `-` after the comment marker (`# -`), only for genuine parallel sub-items + hanging off a lead line, never a continuation of one thought. +- **No file, class, or type header summary blocks.** A type or file gets a comment only for a + specific non-obvious point, never a block restating what it contains (a license or provenance + header a tool or policy requires is not a summary and is unaffected). +- **Never let a comment grow across edits.** Touching code near an existing comment means the + comment comes out the same length or shorter, never one more clause of rationale appended. + +A continuation stays unindented, one sentence per line: + +```text +# Change gate for the compile tests. +# An esp-idf build costs minutes, so gate on what each test covers. +# A diff that cannot be computed runs everything. +``` + +Sub-topics take a `-` after the comment marker, each elaborating a distinct item named in the lead: + +```text +# Source lint plus change-gated compile tests. +# - compile-test builds the external component. +# - template-compile-test builds one example device per template. +``` + +## Character set + +Agent-authored text is ASCII by default: documentation, code, comments, commit messages, and PR +descriptions. A non-ASCII character is read against three tiers, because whether one is +typography or meaning depends on where it sits. A character in no tier is a finding rather than a +silent pass. + +- **Tier 1, never legitimate.** Typography carrying no meaning its ASCII form loses. Remove on + sight: + - em dash (U+2014) and en dash (U+2013) to a restructured sentence, two sentences or a comma, + never a spaced hyphen + - right arrow (U+2192) to `->`, double arrow (U+21D2) to `=>` + - curly quotes (U+2018/U+2019/U+201C/U+201D) to straight `'` and `"` + - ellipsis (U+2026) to `...`, bullet (U+2022) to `-` + - no-break space (U+00A0) to a space, non-breaking hyphen (U+2011) to `-` +- **Tier 2, legitimate only next to a number.** Relational and arithmetic operators: U+2264, + U+2265, U+2260, U+00B1, U+2212, U+00D7, U+00F7, U+00B7. Keep one when an adjacent non-space token + is a number, a tier-3 symbol, or another tier-2 operator, so a threshold table or a measured + range reads as the range it is. In flowing prose write the ASCII form: `<=`, `>=`, `!=`, `+/-`, + `-`, `x`, `/`. A tier-2 operator directly before a number in a table of thresholds is the range + it describes and stays, the same character between two words in a sentence is prose and takes + the ASCII form. +- **Tier 3, always legitimate.** Scientific and unit symbols whose ASCII form would be a lie: + micro (U+00B5), degree (U+00B0), ohm (U+2126), pi (U+03C0), superscript two and three (U+00B2, + U+00B3), section (U+00A7). Keep the symbol, never approximate it away or spell it out. +- **Unicode a developer deliberately typed** stays regardless of tier, such as emoji used for + emphasis or as callout markers. Never strip a developer's own characters, this is developer + authored text and not a license for the agent to add its own. +- **An unrecognized non-ASCII character is reported, not allowed.** Classify it into a tier above + before using it. +- **No semicolon in agent-authored prose.** Recast a mid-sentence semicolon as a comma or as two + sentences. A semicolon separating items in a list that already contains commas, or a statement + terminator in code, is unaffected. +- **No spaced hyphen joining or interrupting a sentence** (` - `, or the paired aside ` - x - `). + Recast as a comma, two sentences, or parentheses. A hyphen inside a compound word, a leading + list marker, a range, and the `- **Label** - explanation` bullet separator are unaffected. +- **In carried verbatim content, fix the whole class at the hub**, not one instance, since a + downstream repo cannot edit a section byte-matched against the hub. Everywhere else, correct as + each file is next edited, not swept. + +## Line endings + +This repo's default is LF (`[*] end_of_line = lf` in `.editorconfig`), with CRLF pinned only for +`*.bat` and `*.cmd`, the one type Windows itself requires it for. +**Preserve a file's existing line ending when editing it, never reflow as a side effect of a +content change.** A text-mode tool, including a naive programmatic write, can silently flip CRLF +to LF and turn a one-line change into a whole-file diff. After any programmatic edit, verify with +`git diff --stat` (it should touch only the lines you changed) and a byte scan, `file` and a naive +`git ls-files --eol` are both unreliable here. Idempotent normalize: +`b.replace(b"\r\n", b"\n").replace(b"\n", b"\r\n")`. The full policy, choosing an ending for a new +file type, operational-repo overrides, extensionless-script pins, and auditing, is in +`references/line-endings.md`. + +## Carried files reference no coordination machinery + +`AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`, `WORKFLOW.md`, `.github/copilot-instructions.md`, +the `spec/` files and the carried `AUDIT.md` never reference the template repo +(in prose or a link), and never name a sibling fleet repo as an illustrative example. State the +behavior a carried rule needs, not the coordination flow that produced it, the maintainer supplies +the destination out of band. A contextually relevant link to a related project (the image this +config feeds, a library this depends on) is not a coordination reference and is expected. The full +exceptions, a verbatim section that must name the hub to do its job, and a pointer to a +hub-hosted tool the reader runs, are in `references/carried-doc-references.md`. + +## PR titles and commit messages + +- **Format**: an imperative subject, 72 characters or fewer, no trailing period ("Add 24-Hour + PM2.5 Average Sensor", not "Added X" or "Adds X"). An optional body, blank-line separated, + explains *why* the change is being made when that is non-obvious, the diff already shows *what*. +- **Rules**: no vague titles (`update stuff`, `wip`). Dependabot's default `Bump X from Y to Z` + titles are fine as-is. No `Co-Authored-By:` lines unless the developer explicitly asks. No + release-bump magnitude in the title ("minor", "patch", "release v0.2.0"), Nerdbank.GitVersioning + computes the next version from `version.json` and git history, a dependency version in a + dependency-bump title is fine and expected. US English spelling, and title case with lowercase + short bind words (a, an, the, and, but, or, of, in, on, at, to, by, for, from), a hyphenated + compound capitalizes both parts unless the second is a short preposition (*Built-in*, + *EPA-Corrected*, *24-Hour*). + +```text +Add Structured Logging Extensions to Library +Pin softprops/action-gh-release to Commit SHA +Drop net8.0 Multi-Targeting from Console Project +Bump xunit.v3 from 3.2.2 to 3.3.0 +Clarify devcontainer Setup Steps in README +``` + +## Quantitative claims + +A quantitative claim in `README.md` (a count, a size, a version floor, a supported-platform list) +is verified against current code before it is written. When a doc number is derived from a code +constant, mark the dependency in a source-code comment so the next editor knows to update both. diff --git a/.github/skills/comment-and-doc-style/references/carried-doc-references.md b/.github/skills/comment-and-doc-style/references/carried-doc-references.md new file mode 100644 index 0000000..125bf0f --- /dev/null +++ b/.github/skills/comment-and-doc-style/references/carried-doc-references.md @@ -0,0 +1,61 @@ +# Carried Files Carry No Coordination References + +Full detail for the "Carried files reference no coordination machinery" rule in `SKILL.md`. Load +this when editing one of the carried files themselves, not when writing an ordinary repo-owned +doc. + +## Which files this governs + +`AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`, `WORKFLOW.md`, `.github/copilot-instructions.md`, +the `spec/` files and the carried `AUDIT.md`, the files the fleet carries +verbatim or at `intent` fidelity from the hub into every repo. This rule governs carried template +content only. A repo's own `README.md` and topical docs are its own content, never carried +verbatim, and this rule does not reach them. + +## What is banned + +Two things, in the files above: + +1. **Any reference to the template repo**, in prose or in a link. The coordination flow that + produced a carried file is machinery a consumer of that repo should never have to see, and + naming where a file came from is exactly the derived-from framing the present-tense rule (in + `SKILL.md`'s "Markdown formatting" section) independently forbids. Where a carried file must + express a template-level behavior ("report a rule discrepancy upstream"), state the behavior + rather than the destination. The maintainer supplies the destination out of band. +2. **A sibling fleet repo named as an illustrative example** ("repo X does it this way", "see repo + Y's adoption"), which couples the repos and rots as they diverge. To point at a current good + example, name it in the onboarding or conformance issue, never in a carried doc. + +## The two exceptions + +**The first exception is a verbatim section**, and `AGENTS.md` "Fleet Bootstrap" is why it exists. +That section's whole function is to name where the canonical rules live, for an agent in a +repository whose carried copies are stale, partial, or absent, which is exactly when no other file +present can say it. Its bytes are fixed fleet-wide, so a repository cannot edit the reference out +without failing the verbatim check instead, and a rule banning it would be unsatisfiable rather +than merely strict. The exception is scoped to the verbatim region and never leaks past it: the +same document's own prose, outside that region, is governed normally. A reference that reaches a +verbatim section is a defect in the canonical, fixed once at the source rather than reported +against every repository carrying it. + +**The second exception is a hub-hosted tool the reader is told to run**, which is a different kind +of reference. A rule naming a gate, a script, or a reference snippet the reader executes or copies +states an instruction rather than a provenance, and an instruction with no destination is +unfollowable, which is precisely how a pointer in carried text comes to read as decorative. The +test is whether the reference is something the reader *does* or something that *happened to this +file*: where the content came from stays out, what the reader runs stays in. Such a pointer names +the hub's canonical rather than this repository's provenance, so it is the hub's to keep resolving +and never a repository's to edit out or re-point at a local path. What is reached rather than +carried, and how, is `GOVERNANCE.md` "Hub-Hosted Tooling". In `AGENTS.md` and `GOVERNANCE.md` this +belongs in verbatim rule text, the same region the first exception already covers, so the whole +fleet reads one wording and no repository is asked to answer for a reference it did not write. + +## What is not a coordination reference + +**A contextually relevant link to a related project is expected, not banned.** Where another repo +is part of this repo's subject matter (the image that consumes this config, the builder that +generates this hardware, a library this depends on), link it normally. The test is whether the +link serves a reader of *this* repo's content, not whether the target happens to be in the fleet. + +This pairs with the present-tense rule: state the current shape, not a history of which repo it +came from. diff --git a/.github/skills/comment-and-doc-style/references/line-endings.md b/.github/skills/comment-and-doc-style/references/line-endings.md new file mode 100644 index 0000000..8337cbc --- /dev/null +++ b/.github/skills/comment-and-doc-style/references/line-endings.md @@ -0,0 +1,117 @@ +# Line Ending Policy + +Full detail for the "Line endings" rule in `SKILL.md`. Load this when choosing an ending for a +new file type, working in an operational (config) repo, pinning an extensionless executable, or +auditing a repo's endings, not for an ordinary content edit to an existing file (the SKILL.md +summary, preserve the existing ending and verify with a byte scan, covers that case). + +## The defaults + +- **`.editorconfig` sets the line ending.** `[*] end_of_line = lf` is the default, every file + type is LF unless pinned otherwise, with CRLF pinned for the one exception Windows requires: + `*.bat` and `*.cmd` (cmd.exe's line handling is unreliable on LF). Only the CRLF exception is + declared, the redundant per-type LF rules are intentionally omitted, since the default already + gives shell scripts, Dockerfiles, workflow YAML, `uv.lock`, and every shebang-executed `.py` + the ending they need without a path-specific pin. +- **`.gitattributes` mirrors the repository-wide defaults**: `* text=auto eol=lf` normalizes every + detected text file to LF while leaving binary files byte-preserved. `*.bat` and `*.cmd` override + that default to CRLF. Do not add per-language or per-file LF pins where the global LF default + already applies. The CRLF-native exception for POSIX-executed paths is defined below. +- **Both files are required together.** `.editorconfig` governs the editor, `.gitattributes` + governs git (checkout, commit, `--renormalize`). A repo missing either file, or whose + `.editorconfig` sets no global `end_of_line` default (for example declares it only under + `[*.md]`), accumulates files mixed between LF and CRLF, the exact failure these two files + prevent together. Carry both files whole. An inert `[*.cs]` block costs nothing in a non-.NET + repo. + +## Choosing an ending for a new file type + +LF is the default, since it is what every tool, CI runner, and Dependabot bump produces, and +Windows GUI editors (VS Code, Visual Studio, Notepad, WordPad) all read and write it cleanly. Pin +CRLF only for a type Windows itself requires it for: `*.bat` and `*.cmd`. Everything else, +including YAML (workflow and non-workflow alike, no distinction needed now that both are LF), +`.gitignore`, `.dockerignore`, and a tool-owned format with a native LF ending (KiCad), takes the +`[*]` default with no override. + +## Operational (config) repos + +The global default follows the consuming application's native platform, not the fleet LF default. +A config repo (registry `workflowModel: operational`) is a view into an application's +configuration directory, often the exact tree mounted into that app's container, so its files use +the ending the app itself reads and writes, and forcing the fleet LF default would fight an app +that needs CRLF. Set the `[*] end_of_line` default to the app's native ending and record it in the +registry `lineEndings` field (`lf` or `crlf`): the field is required for every operational repo +because a config repo's ending is a load-bearing decision tied to its consuming app. A +Linux-native app or container config uses the `lf` fleet default. A Windows-native app that uses +CRLF requires `[*] end_of_line = crlf` and +`* text=auto eol=crlf`. `release` repos keep the LF fleet defaults above. Do not re-normalize an +operational repo to +the fleet default, that is exactly the over-normalization these per-repo endings exist to +prevent, whichever direction the fleet default currently points. + +**Mixed-consumer config: prefer to split by platform into single-platform repos, not one mixed +repo.** When a config repo would be consumed on two platforms (a Linux app plus a Windows-edited +subtree), the clean answer is a repo per consumer, each single-platform with its own +`lineEndings`. For example a controller config edited by a Windows-native editor lives in its own +CRLF repo, not as a subtree inside a Linux `lf` config repo. Fallback only if a subtree genuinely +cannot be split out: keep the global default at the primary consumer and pin the odd subtree with +an `.editorconfig` path override (for example `[<subtree>/**] end_of_line = crlf`) matching its +consumer. Pair the same path override in `.gitattributes`, since both layers must resolve the +path to the same ending. + +## Scripts and extensionless executables + +Must be LF. A CRLF shebang (`#!/usr/bin/env bash\r`) breaks execution. The paired global LF +defaults cover extensionless executables, shell scripts, and directly executed Python without +path-specific pins. A CRLF-native operational repo adds narrow matching LF overrides in both +files only for scripts it executes on POSIX. + +For a type that genuinely needs an ending the `[*]` default no longer supplies (a Windows-native +tool-owned format outside `.bat`/`.cmd`, or a byte-preserve data directory whose exact bytes the +consumer may depend on), still pair a `.gitattributes` pin with a matching `.editorconfig` +override, since the git pin alone is not enough there, `.gitattributes` governs git while the +editor follows `.editorconfig`. For a byte-preserve directory, disable all editor normalization, +not just EOL: `[<dir>/**]` with `charset = unset`, `end_of_line = unset`, `insert_final_newline = +false`, `trim_trailing_whitespace = false` (`unset` is EditorConfig's spec-defined special value +that removes an inherited property, and `**` is needed rather than `*` so a nested file under the +directory is covered too, since `*` excludes `/` and only matches one path component). + +## Editing discipline + +- **New files**: create with the `.editorconfig`-mandated ending. +- **Editing an existing file**: preserve its current line endings, do not reflow them as a side + effect of a content change, even if the file is already non-compliant. A tool that rewrites a + file in text mode (a script, a bulk find/replace) can silently flip CRLF to LF and turn a + one-line change into a whole-file diff. After any programmatic edit, verify before staging: + `git diff --stat` should touch only the lines you changed, and a byte check should confirm the + expected ending. If a diff balloons to the whole file, the endings flipped, restore them and + re-stage. +- **Fixing a non-compliant file**: bring it to its `.editorconfig` ending as a deliberate change, + and prefer to isolate it in its own EOL-only commit so the churn is reviewable. When a broader + maintenance change has to normalize endings alongside content edits, call it out explicitly in + the commit or PR description and verify the content separately with + `git diff --ignore-cr-at-eol`. + +## Auditing + +Don't trust `file` or a naive `git ls-files --eol`. The authoritative check is a byte scan that +classifies by which endings are present: CRLF-only (every `\n` preceded by `\r`), LF-only (no +`\r`), or mixed (both forms present). Flag mixed explicitly rather than lumping it in with CRLF, +and skip binaries via a NUL-byte check. `file` mislabels some types (it reports a CRLF `.json` or +`.code-workspace` as plain "JSON text data" with no CRLF note), and `git ls-files --eol`'s `attr/` +column holds multiple tokens that shift naive field-splitting into false positives. Scope a +repo-wide audit to `git ls-files` plus `git ls-files --others --exclude-standard`, never a raw +`find`, which sweeps self-ignoring caches (`.mypy_cache`, `.artifacts`). + +Idempotent normalize: `b.replace(b"\r\n", b"\n").replace(b"\n", b"\r\n")`. A single within-line +string replace is EOL-safe, but a tool that inserts multiple lines or writes a new file into a +CRLF file must emit `\r\n`, since a naive `\n` insert creates mixed endings. `.code-workspace` is +JSONC (it has `//` comments), so strip them before JSON-parsing it. + +Editing CRLF files programmatically with a regex has a sharper trap: `.` matches `\r`, so a +captured line keeps its carriage return and rejoining with `\r\n` yields `CRCRLF`. A text-mode +rewrite has the mirror failure, silently flattening CRLF to LF. Prefer line-based edits +(`splitlines(keepends=True)`) or literal replacement over regex reassembly. In Python the +text-mode failure is the default: `Path.read_text()` decodes through universal newlines and +`write_text()` writes `\n` back, so a read-edit-write round trip flattens the whole file while the +edit itself looks correct. Pass `newline=''` to both, or work in bytes. diff --git a/.github/skills/comment-and-doc-style/references/markdown-links.md b/.github/skills/comment-and-doc-style/references/markdown-links.md new file mode 100644 index 0000000..c3cd7e1 --- /dev/null +++ b/.github/skills/comment-and-doc-style/references/markdown-links.md @@ -0,0 +1,64 @@ +# Reference-Style Links + +Full detail for the "Markdown formatting" reference-style-links rule in `SKILL.md`. Load this +when actually authoring or reorganizing a Markdown file's link definitions, not for a small +in-place prose edit. + +## Where the rule applies + +Every Markdown file in the repo uses reference-style links only, except the four files that are +read one section at a time rather than end to end: `AGENTS.md`, `GOVERNANCE.md`, `OPERATIONS.md`, +and `.github/copilot-instructions.md`. Those keep inline `[text](uri)` links, since a reader +jumping straight to one section needs the target to resolve where it is, while a definition parked +at the bottom of the file is never reached. The exception is that closed list of four files, never +a category to argue from case by case. Every other Markdown file follows the rule regardless of +its audience. + +## The definition block + +Every URI, an internal path, an anchor, an external URL, or a shield image, is defined at the +bottom of the file, split into groups by type under an HTML-comment header, for example: + +```markdown +<!-- Shields --> + +[license-shield]: https://img.shields.io/... + +<!-- Repo --> + +[governance]: ./GOVERNANCE.md +[governance-branching-model]: ./GOVERNANCE.md#branching-model + +<!-- External --> + +[markdownlint-cli2]: https://github.com/DavidAnson/markdownlint-cli2 +``` + +Within a group, definitions are alphabetized by **reference name alone**, the text inside the +brackets, never by the whole definition line. Where one name is a prefix of another, the shorter +one sorts first: `[governance]` above `[governance-branching-model]`, `[repo-config]` above +`[repo-config-settings]`. Sorting the full line instead inverts every such pair, because `-` +precedes `]` in byte order, so the two readings disagree on exactly the names a reader looks up +together, and a plain `sort -c` over the block passes on the inverted order regardless. + +## Naming a reference + +Reference names are contextual and encode both the target and its group: + +- `foo-shield` for a shield image +- `foo-link` for an external URL +- a bare `foo` for a local path or anchor + +For example `[license-shield]`, `[releases-link]`, `[repo-config]`. Never a numeric name (`[1]`) +and never an opaque one. + +## Mechanics + +- No inline `[text](uri)` targets in prose, in any file outside the four-file exception above. +- **A URL inside a fenced code block stays inline.** Reference links do not resolve inside a code + block, so do not extract it there, and exclude fenced code from any link-integrity check + (bracket literals like `["a", "b"]` otherwise read as undefined references). +- **Removing a link also removes its reference definition.** An orphaned definition fails the + no-unused-defs rule. +- The one exception to "no inline links" is the Table of Contents, whose entries stay inline + anchor links, since the ToC extension generates them that way and they are never hand-edited. diff --git a/.github/skills/copilot-instructions-keeper/SKILL.md b/.github/skills/copilot-instructions-keeper/SKILL.md new file mode 100644 index 0000000..9ee43bc --- /dev/null +++ b/.github/skills/copilot-instructions-keeper/SKILL.md @@ -0,0 +1,95 @@ +--- +name: copilot-instructions-keeper +description: >- + Helps keep a repo's .github/copilot-instructions.md in sync with the ptr727/ProjectTemplate hub + canonical, and stops the one mistake specific to this file: silently wiping its repo-local + "Disproved Claims" ledger entries during a resync. Use this whenever about to edit, overwrite, + re-vendor, or carry .github/copilot-instructions.md into a repo, whenever checking a repo for + drift against the hub or running a conformance sweep that touches this file, whenever GitHub + Copilot's review mechanics in this file look stale, wrong, or missing something the fleet + runbook should cover, or whenever standing up a new repo and carrying this file for the first + time. Also triggers on "why isn't the audit catching that this file is out of date," since the + fleet's mechanical audit checks this file, at intent fidelity, for file presence and each named + section's heading, never for content drift inside a section, so nothing else notices a stale + section here except a live check like this one. +--- + +# Copilot Instructions Keeper + +## Why this exists + +`.github/copilot-instructions.md` is read directly by GitHub Copilot and bootstraps the shared +`AGENTS.md` instruction set and review-focused skills. Its Copilot-specific rules stay fully +intact in every repo that carries it. This skill maintains that carried copy, it does not replace +the bootstrap. + +`spec/files.json` declares it `intent` fidelity, `whole: true`, covering four named sections +(`Commit Messages and Pull Request Titles`, `Reviewing Carried Fleet Content`, `GitHub Copilot +Review Runbook`, `When in Doubt`), with `<owner>`, `<repo>`, and `<N>` placeholders filled per +repo. **The fleet audit checks an `intent` file for file presence and each named section's +heading, never for content drift inside a section.** A section that is present but has fallen out +of date against the hub, the exact gap this skill exists to catch, produces no finding anywhere in +the mechanical audit. Noticing that has to happen in a live session like this one. + +## The one thing this file has that others don't: repo-local ledger entries + +The file's own "Disproved Claims" section states its rule plainly. **The section's shape and +governing rules are carried, but its entries are not.** Each entry records a finding that was +raised against this specific repository and disproved against this repository's code at a named +revision. A repository carrying a copy of this file carries the shape and rules, deletes any +entry whose subject it does not hold, and records what it has proved for itself. + +This means a blind re-vendor of the hub's canonical `.github/copilot-instructions.md` over a downstream +repo's copy is wrong in both directions: + +- Copying the hub's own "Disproved Claims" entries (about `ProjectTemplate` itself) into a + downstream repo attaches proofs about code that repo does not carry. +- Overwriting a downstream repo's copy wholesale deletes any entries that repo itself has earned, + a live disproof, run against that repo's own tree, thrown away with no record. + +**Before touching this file in any repo other than the hub itself:** + +1. Read the current "Disproved Claims" section in that repo's copy, if it has one, and preserve + every entry that names a file or behavior that repo actually carries. +2. Update everything else, the runbook mechanics, the four named sections, the rule text, to + match the hub canonical. +3. Never carry the hub's own repo-specific "Disproved Claims" entries downstream. They name + `ProjectTemplate`'s own files and revisions, not the target repo's. +4. If in doubt whether an entry is still valid for the current tree, treat it per the guard skill + below rather than guessing. + +The `carried-instruction-file-guard` skill stops this same failure class for `AGENTS.md`, +`GOVERNANCE.md`, `CODESTYLE.md`, and `WORKFLOW.md`: a +routine-sounding overwrite silently deleting content that is not a stale copy of the hub. Run +that skill's distinctive-phrase probe against this file too before any full-file replace. It is +not in that skill's own file list because its failure mode, ledger entries rather than fleet +rules, is specific enough to warrant its own skill, but the underlying discipline, probe before +overwrite, give a local addition a destination rather than deleting it, is the same. + +## Checking a repo's copy for drift + +1. Fetch the hub (`github.com/ptr727/ProjectTemplate`) `main` branch fresh. A stale local clone + answers confidently instead of failing. +2. Compare the target repo's `.github/copilot-instructions.md` against the hub's, section by + section, at **intent** fidelity, judged by meaning, not by byte match. A content-identical + file with different `<owner>`/`<repo>` placeholder fills is current, not drifted. +3. Read the "Disproved Claims" section separately from the rest. Judge its **shape and rules** + against the hub, and judge its **entries** only against what that repo itself carries (see + above), never against the hub's own entries. +4. Report what is actually stale (a runbook mechanic that changed, a rule that moved, a new + section) versus what only looks different because it is correctly repo-specific. + +## Carrying it fresh, new repo or full resync + +Follow `RESYNC.md`'s general apply order for carried files, with the ledger rule above applied at +the point this file is touched: carry the hub's current rule text and runbook mechanics, keep the +target repo's own "Disproved Claims" entries (if any existed pre-resync) rather than replacing +them with the hub's, and start a new repo's ledger empty rather than seeded from the hub's own +proofs. + +## What this skill does not cover + +Content-style rules for other carried files (`AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`, +`WORKFLOW.md`) are `carried-instruction-file-guard`'s job. The review-loop contract this file's +runbook implements, the merge gate, triage, escalation, is `pr-review-conduct`'s job. This skill +is narrowly about keeping this one file's carried copy correct. diff --git a/.github/skills/dotnet-codestyle/SKILL.md b/.github/skills/dotnet-codestyle/SKILL.md new file mode 100644 index 0000000..f3565a5 --- /dev/null +++ b/.github/skills/dotnet-codestyle/SKILL.md @@ -0,0 +1,211 @@ +--- +name: dotnet-codestyle +description: >- + Governs C#/.NET code style for ptr727/ProjectTemplate fleet repos: the zero-warnings build + policy and its three-task clean-compile chain, central Directory.Build.props/ + Directory.Packages.props configuration, C# language and naming conventions, XML documentation, + analyzer suppression scope, the library-versus-application logging split, async and + error-handling patterns, xUnit v3 + AwesomeAssertions testing conventions, and AOT-compatible + project configuration. Use this whenever writing, reviewing, or editing a .cs file, a .csproj, + Directory.Build.props, or Directory.Packages.props, whenever choosing where to suppress an + analyzer diagnostic, whenever a NuGet library needs to log without depending on Serilog + directly, or whenever writing or reviewing an xUnit test. Triggers even when the task looks + like a small local fix ("just silence this warning", "add a quick log line", "bump a package + version"), because the zero-warnings policy, the suppression-scope order, the central-package- + management rule, and the library/application logging split are each easy to violate one file at + a time without the pattern ever showing up as a single obvious diff. Applies only to a repo's + .NET side, a repo with no .NET projects has no use for this Skill. +--- + +# .NET Codestyle + +## Why this exists + +This is the .NET-specific half of the fleet's code style guide, kept in one place instead of +re-derived per repo or per session. CODESTYLE.md's General section still owns the rules every +language shares (clean-compile verification as a concept, the suppression-scope order, tooling +casing in prose), this Skill is everything specific to a C#/.NET project on top of that: the +concrete `.NET Format` task chain, the analyzer configuration that makes the zero-warnings policy +real, and the language, naming, logging, and testing conventions. + +## Build requirements + +### Zero warnings policy + +All builds must complete without warnings, enforced three ways: + +- **The `.NET Format` clean-compile task.** It chains `CSharpier Format` -> `.NET Build` -> + `dotnet format style --verify-no-changes`. A repo carries those three task definitions in its + own `.vscode/tasks.json`, matching the canonical `vscode-tasks.json` snippet at + `github.com/ptr727/ProjectTemplate/blob/main/catalog/snippets/configs/vscode-tasks.json`. Run + the `.NET Format` task after any code change, before commit. To run it natively instead, + reproduce that exact task chain (`CSharpier Format`, then `.NET Build`, then + `dotnet format style --verify-no-changes --severity=info --verbosity=detailed`) without dropping + or loosening any argument, reading it from that same canonical snippet. Bare `dotnet format` + alone, skipping CSharpier or the build, is not sufficient. +- **Analyzer configuration.** `<EnableNETAnalyzers>true</EnableNETAnalyzers>` with + `<AnalysisLevel>latest-all</AnalysisLevel>` and `<AnalysisMode>All</AnalysisMode>` (the full + analyzer set), plus `<TreatWarningsAsErrors>true</TreatWarningsAsErrors>`, so any diagnostic + surfaced as a warning fails the build and must be fixed or deliberately suppressed at the + narrowest scope that fits (see Analyzer suppressions below), never left to accumulate. +- **CI lint backstop.** CI runs the clean-compile checks on every PR as the authoritative gate. + A working local hook is strongly suggested, not optional: wire Husky.Net from the canonical + `catalog/snippets/husky/` config. See GOVERNANCE.md "Running the Linters Locally" for what the + hook must cover and what its absence means. + +**A new port is not a license to silence diagnostics.** Brownfield or just-ported status never +justifies relaxing analyzer severities or muting newly surfaced warnings. Fix them. (The only +brownfield allowance in the fleet is the one-time git-signing / line-ending migration described in +GOVERNANCE.md and README.md, which has nothing to do with code analysis.) + +### Central build and package configuration + +Shared MSBuild configuration is centralized at the repository root, never duplicated per project: + +- **`Directory.Build.props`** carries the properties every project shares: the analyzer set and + `TreatWarningsAsErrors` from the zero-warnings policy above, plus `LangVersion`, + `TargetFramework` where uniform, and any repo-wide build metadata. A `.csproj` carries only what + is genuinely project-specific (`OutputType`, `IsPackable`, project references). +- **`Directory.Packages.props`** owns central package management: it sets + `ManagePackageVersionsCentrally` to `true` (in this file, not `Directory.Build.props`) and + declares every dependency version once as a `PackageVersion` item, so a `.csproj`'s + `PackageReference` items are versionless. One file to review on a bump, one Dependabot surface, + no version skew between projects. + +A repo whose projects still carry per-project analyzer settings or versioned `PackageReference` +items is drifted, move the shared property or version up to the root file rather than editing it +in place. + +### Build tasks + +Run these from VS Code's task runner (Terminal -> Run Task) or an agent's task-running tool. The +three clean-compile tasks are carried verbatim, and a repo adds its own convenience tasks (tool +updates, dependency upgrades, benchmarks) on top: + +- `.NET Build`: build with diagnostic verbosity *(clean-compile)* +- `CSharpier Format`: auto-format code with CSharpier *(clean-compile)* +- `.NET Format`: run CSharpier and build, then verify formatting and style with + `--verify-no-changes` *(clean-compile, the task to run after edits)* + +## Tooling and editor + +- **CSharpier** is the primary code formatter, invoked by the `CSharpier Format` task or + `dotnet csharpier format --log-level=debug .`. +- **`dotnet format`** verifies style: + `dotnet format style --verify-no-changes --severity=info --verbosity=detailed`. +- **`dotnet-outdated-tool`** checks for dependency updates, and Nerdbank.GitVersioning owns + version management. +- CI is the authoritative lint backstop. A local pre-commit hook is strongly suggested: wire + Husky.Net from `catalog/snippets/husky/` for local enforcement, including the shared doc gates. +- **Required VS Code extensions**: CSharpier, markdownlint, CSpell. Use the workspace settings + without overrides. + +## Coding standards and conventions + +Key rules: no `var` (always explicit types), file-scoped namespaces, Allman braces, Nullable +enabled, modern C# features (primary constructors, pattern matching, collection expressions). Every +public surface has XML documentation. Private fields use `_camelCase`, static fields `s_camelCase`, +constants PascalCase. Member ordering follows StyleCop SA1201. + +For language features, naming, code structure, and XML documentation examples, see +`references/conventions.md`. + +## Analyzer suppressions (.NET) + +CODESTYLE.md's General section sets the suppression-scope order fleet-wide: narrowest scope first, +symbol-scoped before project-scoped before repo-wide, and only for a genuine false-positive or a +deliberate, documented exception, never a blanket relaxation to get a brownfield port to build. +The .NET mechanics, narrowest first: + +- **Never use `#pragma warning disable`** to silence an analyzer. +- **Symbol-scoped**: a `[System.Diagnostics.CodeAnalysis.SuppressMessage(...)]` attribute with a + `Justification`, on the specific member or type: + + ```csharp + [System.Diagnostics.CodeAnalysis.SuppressMessage( + "Design", + "CA1034:Nested types should not be visible", + Justification = "https://github.com/dotnet/sdk/issues/51681" + )] + ``` + +- **Project-scoped** (e.g. a test project): a `dotnet_diagnostic.<RULE>.severity` entry in that + project's own `.editorconfig`, with a comment explaining why. +- **Repo-wide**: a `dotnet_diagnostic.<RULE>.severity` entry in the root `.editorconfig`, only + when the rule is genuinely not applicable to any project. Relaxing a batch of `CA*` rules (or + `dotnet_analyzer_diagnostic.severity`) to push a brownfield port through the build is exactly + what this forbids. + +## Error handling and logging + +1. **Structured logging**: use structured message templates. Serilog is the application's concrete + backend, and a library never references it directly (see item 2): + + ```csharp + logger.LogError(exception, "{Function}", function); + ``` + +2. **Libraries log through abstractions, never a concrete backend.** A NuGet library depends only + on `Microsoft.Extensions.Logging.Abstractions` and exposes an `ILoggerFactory` seam: a settable + global factory defaulting to `NullLoggerFactory.Instance` (fallback `NullLogger.Instance`) with + `SetFactory`/`TrySetFactory`, and/or an `ILoggerFactory`/`ILogger` parameter in its API. It + must not reference Serilog or any sink, which would force a logging framework on every consumer + and drag in AOT-incompatible dependencies. The consuming application owns the concrete logger + (Serilog is fine there), bridges it to `ILoggerFactory` (e.g. `SerilogLoggerFactory` from + `Serilog.Extensions.Logging`), and injects it. Reference pattern: a `LogOptions` seam in the + library, against which the consuming CLI builds the Serilog-backed factory and injects it via + `LogOptions.SetFactory`. +3. **CallerMemberName**: use for automatic function name tracking: + + ```csharp + public bool LogAndPropagate( + Exception exception, + [CallerMemberName] string function = "unknown" + ) + ``` + +4. **Logger extensions**: use `Extensions.cs` for logger and other extension methods: + + ```csharp + extension(ILogger logger) + { + public bool LogAndPropagate(Exception exception, ...) { } + } + ``` + +5. **Exceptions**: do not swallow exceptions, either log and rethrow or translate to a + domain-specific exception. + +## Code patterns + +1. **Guard clauses**: prefer early returns for validation and error handling. +2. **Async all the way**: avoid blocking calls (`.Result`, `.Wait()`), use `async`/`await`. +3. **Cancellation tokens**: accept `CancellationToken` as the last parameter and pass it through. +4. **ConfigureAwait**: in library code, use `ConfigureAwait(false)` unless context is required. Do + not call `ConfigureAwait(false)` in xUnit tests (see xUnit1030). +5. **Disposables**: use `await using` for async disposables, prefer `using` declarations. +6. **LINQ vs loops**: use LINQ for clarity, loops for hot paths or allocations. +7. **HTTP**: reuse `HttpClient` via factory, never per-request instantiation. +8. **Collections**: prefer `IReadOnlyList<T>`/`IReadOnlyCollection<T>` for public APIs. +9. **Immutability**: prefer immutable records, use init-only setters when records are not + suitable, and prefer immutable or frozen collections for read-only data. +10. **Exceptions as control flow**: avoid using exceptions for expected flow. +11. **Sealing classes**: seal classes that are not designed for inheritance. +12. **Lazy initialization**: use `Lazy<T>` for static, thread-safe instantiation (e.g. a logger + factory, an HTTP factory). + +## Testing conventions + +xUnit v3 (`xunit.v3`, not the legacy `xunit`) + AwesomeAssertions (`.Should()` API, never native +asserts). Arrange-Act-Assert pattern, descriptive underscore names, `[Theory]`/`[InlineData]` for +parameterized tests. See `references/testing.md` for the framework setup template. + +## Project configuration + +.NET 10.0 target, AOT-compatible (`IsAotCompatible=true`, `VerifyReferenceAotCompatibility=true`), +SourceLink, embedded untracked sources, `InternalsVisibleTo` for test/benchmark access. See +`references/project-config.md` for the full property list. + +## Best practices + +All changes go through pull requests. diff --git a/.github/skills/dotnet-codestyle/references/conventions.md b/.github/skills/dotnet-codestyle/references/conventions.md new file mode 100644 index 0000000..46897c8 --- /dev/null +++ b/.github/skills/dotnet-codestyle/references/conventions.md @@ -0,0 +1,136 @@ +# .NET Coding Standards and Conventions + +Code snippets below are illustrative examples only, replace namespaces and types to match your +project. + +## C# language features + +1. **File-scoped namespaces**: + + ```csharp + namespace Example.Project.Library; + ``` + +2. **Nullable reference types**: enabled (`<Nullable>enable</Nullable>`), use nullable annotations + appropriately, use `required` for mandatory properties. +3. **Modern C# features**: prefer modern language constructs, primary constructors when + appropriate, top-level statements for console apps, pattern matching over traditional checks, + collection expressions when types loosely match, extension methods (the classic + `this`-parameter form or an `extension(<receiver>) { ... }` block on C# 14+), implicit object + creation when the type is apparent, range and index operators. +4. **Expression-bodied members**: use for applicable methods, properties, accessors, operators, + lambdas, local functions. +5. **`var` keyword**: do NOT use `var`, always use explicit types: + + ```csharp + // Correct + int count = 42; + string name = "test"; + + // Incorrect + var count = 42; + var name = "test"; + ``` + +## Naming conventions + +1. **Private fields**: underscore prefix with camelCase: + + ```csharp + private readonly HttpClient _httpClient; + private int _counter; + ``` + +2. **Static fields**: `s_` prefix with camelCase: + + ```csharp + private static int s_instanceCount; + ``` + +3. **Constants**: PascalCase: + + ```csharp + private const int MaxRetries = 3; + ``` + +## Code structure + +1. **Global usings**: use `GlobalUsings.cs` for common namespaces: + + ```csharp + global using System; + global using System.Net.Http; + global using System.Threading.Tasks; + global using Microsoft.Extensions.Logging; + ``` + +2. **Usings placement**: outside the namespace, sorted with `System` directives first: + + ```csharp + using System.CommandLine; + using System.Runtime.CompilerServices; + using Example.Project.Library; + + namespace Example.Project.Console; + ``` + +3. **Braces**: Allman style: + + ```csharp + public void Method() + { + if (condition) + { + // code + } + } + ``` + +4. **Indentation**: C# files 4 spaces, XML/csproj files 2 spaces, YAML files 2 spaces, JSON files + 4 spaces. +5. **Line endings**: not specified here, governed per repo by `.editorconfig` / `.gitattributes` + per GOVERNANCE.md's "Line Endings" section. +6. **`#region`**: do not use regions, prefer logical file/folder/namespace organization. +7. **Member ordering (StyleCop SA1201)**: const -> static readonly -> static fields -> instance + readonly fields -> instance fields -> constructors -> public (events -> properties -> indexers + -> methods -> operators) -> non-public in same order -> nested types. + +## Comments and documentation + +XML documentation is on: `<GenerateDocumentationFile>true</GenerateDocumentationFile>`, and +missing XML comments for public APIs are suppressed in `.editorconfig`. Every public surface must +still be documented: a single-line summary, additional details in remarks, documented input +parameters, return values, exceptions, and crefs. + +```csharp +/// <summary> +/// Example of a single line summary. +/// </summary> +/// <remarks> +/// Additional important details about usage. +/// Multiple lines if needed. +/// </remarks> +/// <param name="category"> +/// The quote category to request +/// </param> +/// <param name="cancellationToken"> +/// A <see cref="System.Threading.CancellationToken"/> that can be used to cancel the request. +/// </param> +/// <returns> +/// A <see cref="string"/> containing the quote text. +/// </returns> +/// <exception cref="System.ArgumentException"> +/// Thrown when <paramref name="category"/> is not a supported value. +/// </exception> +public async Task<string> GetQuoteOfTheDayAsync(string category, CancellationToken cancellationToken) +{ + if (category is not ("motivational" or "humor")) + { + throw new ArgumentException($"Unsupported category: {category}", nameof(category)); + } + + cancellationToken.ThrowIfCancellationRequested(); + await Task.Delay(1, cancellationToken); + return $"Quote for {category}"; +} +``` diff --git a/.github/skills/dotnet-codestyle/references/project-config.md b/.github/skills/dotnet-codestyle/references/project-config.md new file mode 100644 index 0000000..42b8fd1 --- /dev/null +++ b/.github/skills/dotnet-codestyle/references/project-config.md @@ -0,0 +1,21 @@ +# .NET Project Configuration + +1. **Target framework**: .NET 10.0 (`<TargetFramework>net10.0</TargetFramework>`). +2. **AOT compatibility**: `<IsAotCompatible>true</IsAotCompatible>`, + `<VerifyReferenceAotCompatibility>true</VerifyReferenceAotCompatibility>`. +3. **Assembly information**: use semantic versioning, include SourceLink + (`<PublishRepositoryUrl>true</PublishRepositoryUrl>`), embed untracked sources + (`<EmbedUntrackedSources>true</EmbedUntrackedSources>`). +4. **Internal visibility**: use `InternalsVisibleTo` for test and benchmark access (adapt the + project names to your repo's test/benchmark projects): + + ```xml + <ItemGroup> + <InternalsVisibleTo Include="YourBenchmarkProject" /> + <InternalsVisibleTo Include="YourTestProject" /> + </ItemGroup> + ``` + +5. **Nullable and XML documentation**: `<Nullable>enable</Nullable>`, + `<GenerateDocumentationFile>true</GenerateDocumentationFile>` (see `references/conventions.md` + for the XML documentation format every public surface needs). diff --git a/.github/skills/dotnet-codestyle/references/testing.md b/.github/skills/dotnet-codestyle/references/testing.md new file mode 100644 index 0000000..5a84a17 --- /dev/null +++ b/.github/skills/dotnet-codestyle/references/testing.md @@ -0,0 +1,25 @@ +# .NET Testing Conventions + +1. **Framework**: xUnit v3 or later (the `xunit.v3` package, never the legacy v2 `xunit` package) + with AwesomeAssertions for every assertion. Native xUnit asserts (`Assert.Equal`, + `Assert.True`, ...) are not allowed, use the fluent `.Should()` API. Dynamic test skipping + (`Assert.Skip`, `Assert.SkipWhen`) is control flow, not an assertion, and stays native: + + ```csharp + [Fact] + public void MethodName_Scenario_ExpectedBehavior() + { + // Arrange + int expected = 42; + + // Act + int actual = GetValue(); + + // Assert + actual.Should().Be(expected); + } + ``` + +2. **Organization**: Arrange-Act-Assert pattern. +3. **Naming**: descriptive names with underscores. +4. **Theory tests**: use `[Theory]` with `[InlineData]`. diff --git a/.github/skills/drive-pr/SKILL.md b/.github/skills/drive-pr/SKILL.md new file mode 100644 index 0000000..1e3fb1f --- /dev/null +++ b/.github/skills/drive-pr/SKILL.md @@ -0,0 +1,131 @@ +--- +name: drive-pr +description: >- + Drives a ptr727/ProjectTemplate fleet pull request through its review loop, feature branch into + develop and, when asked, on to a mergeable develop -> main promotion PR, applying the + pr-review-conduct disposition to every reviewer finding along the way: fix it, decline it with + evidence, defer it behind a filed issue, or put the call to the maintainer and wait for an + explicit answer in the same turn. Use this whenever asked to drive, land, take, chase, or push + a PR toward develop or main, or to run the review loop hands off instead of narrating each + round. When the request does not say how far ("drive this PR", "land it"), ask once whether the + target is develop or a mergeable main promotion PR, rather than guessing. Triggers even when + only one PR is named, because a finding raised against the develop -> main promotion PR + routinely needs its own feature -> develop fix cycle before the promotion PR can go green, and + stopping at the first promotion-PR finding is the early exit this skill exists to prevent. Ends + at develop merged, or at a promotion PR meeting the pr-review-conduct Merge Gate, never merges + main itself, that is the separate merge-and-release skill, its own go-ahead. +--- + +# Drive PR + +## Why This Exists + +The same request repeats every time a change is ready: drive it through review, resolve whatever +a reviewer raises, and keep going until develop, or main, actually has it. Re-explaining the +finding-disposition policy and the promotion-PR wrinkle each time is the cost this skill removes. +The wrinkle: a finding raised against the develop -> main promotion PR usually cannot be fixed on +that PR directly, its diff is develop's diff against main, so the fix lands as its own +feature -> develop PR first. Stopping at the first such finding, or forgetting to loop back to the +promotion PR once the fix lands, is the early exit this skill exists to prevent. + +## How Far to Drive + +- Read the invocation for an explicit target first. "To develop" or "to dev" means stop once + merged into develop. "To main", "through to main", or "all the way" means continue to a + mergeable promotion PR. Act on either without asking. +- When the request names no target ("drive this PR", "land it", "take this PR"), ask once, + before the first push: develop only, or all the way to a mergeable main promotion PR. Recommend + "all the way to main" as the default, a promotion PR left to go stale once develop is ready is + the more common regret than driving one step too far. +- A repo on the operational workflow model (registry `workflowModel: operational`) has no + standing promotion PR expectation, confirm whether a promotion PR is even wanted before opening + one, per operational-vs-release-workflow's "Operational repositories" delta. + +## What Invoking This Skill Authorizes + +- Naming this skill, and answering its how-far question, is the maintainer's explicit, current + go-ahead for every feature -> develop squash merge the drive performs to reach that target. +- It is never authorization to merge the develop -> main promotion PR, or to dispatch a release. + Those stay in merge-and-release, invoked on its own so the maintainer keeps a checkpoint before + the harder-to-reverse step. +- The pr-review-conduct Merge Gate still gates every merge this skill performs on its own. The + go-ahead removes the "may I merge to develop" question, not the gate itself, a feature PR with + an open finding does not merge regardless of target. + +## The Drive Loop + +1. Isolate into a worktree per repo-worktree, based on develop, before the first edit. +2. Run `local-strict-review` against the branch's current diff, then push the branch and open + the feature -> develop PR if it does not exist yet. +3. Drive pr-review-conduct's review loop on it to the Merge Gate, disposing of every finding per + "Disposing of Every Finding" below. +4. Capture the branch's own tip before merging, `gh pr view [number] --json headRefOid --jq + .headRefOid`, needed for the verify-then-delete step below since `gh pr merge` itself reports + the resulting squash commit on `develop`, not the PR's `headRefOid`. Merge the feature PR into + develop, `gh pr merge [number] --squash --repo owner/repo`. Never `--delete-branch` on this + call, it is run from inside the task's own worktree per step 1, where the feature branch is + checked out, and `gh pr merge --delete-branch` needs to switch that worktree to the base branch + to delete it, which fails when `develop` is already checked out somewhere else, the ordinary + case in this layout. Instead run repo-worktree's post-merge cleanup from the base clone: remove + the worktree, delete the now-merged local task branch, then verify before deleting the remote + one, `git ls-remote --heads --exit-code -- origin "refs/heads/<branch>"` matches the + `headRefOid` captured above, `--` before `origin` and the fully-qualified ref. `--heads origin + "<branch>"` alone still tail-matches a differently-prefixed branch sharing the same suffix, and + `--` placed after `origin` instead of before it is not equivalent either, verified empirically + against a `refs/heads/other/--` ref: after-origin also matched it, before-origin matched only + the one intended. `--exit-code` distinguishes exit `2`, branch genuinely gone, from any other + non-zero exit, a failed query, an unreachable remote and a gone branch both print nothing to + stdout otherwise. Stop and report either a mismatch or a failed query rather than deleting, + someone could have pushed to the branch after the merge, or the name could have been reused. + `<branch>` is the real value, substituted as its own quoted argument (a shell variable + expansion such as `"$branch"`, or an argv element), never handed to `eval` or `sh -c` for a + second round of shell parsing, the only way an embedded `$()` or backtick would actually run. + A valid ref can start with `-` or carry a shell metacharacter, which is why it stays quoted + regardless. Only once it matches, `git push origin --delete -- "<branch>"`. Never + `--force-with-lease` here, git-commit-conventions forbids it + unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at + delete time. The + repo's auto-delete-head-branches setting is kept off fleet-wide (to protect `develop` and + `main` from it, GitHub has no per-branch exception), so nothing deletes an ordinary feature + branch automatically. Stop here and report the merged PR when the target is develop only. +5. Open the develop -> main promotion PR if it does not exist yet, or find the existing one. +6. Drive its review loop the same way. A finding that needs a code change never gets pushed to + the promotion PR directly, its head is develop, so land the fix as a fresh pass through steps + 1 to 4 in its own worktree and branch, then return here. +7. The fix landing on develop updates the promotion PR's diff and head SHA on its own, re-request + a review on the new head and continue the loop. +8. Repeat 6 and 7 until the promotion PR itself carries no open finding and its checks are green + on the current head. +9. Report the promotion PR number and its ready state. Do not merge it. + +## Disposing of Every Finding + +pr-review-conduct's five outcomes are the actual rule, this is the mapping to use while driving: + +- Real, so fix it. Run `local-strict-review` against the branch's current diff, push it, reply + with its commit SHA (outcome 1). +- Not real, or real but out of scope here, so decline in the thread with evidence: the command + and its output, the code path, or the rule that governs it. An assertion never closes a finding + on its own (outcome 2). +- Real and worth doing, but later, so file the issue first, then reply with its link (outcome 4). +- Real, fixable, but a value call rather than a scope boundary, or the agent genuinely does not + know which of the above applies, so ask the maintainer directly, whatever the runtime's own + interactive-question mechanism is, and get an explicit answer in the same turn, a plan to ask + later is resolution by silence (outcome 3). +- The same finding keeps recurring against correct code, fix the class, sharpen a name, add a + comment, or take the rule itself to the maintainer, rather than re-arguing the instance every + round (outcome 5). + +## Mechanics Live Elsewhere + +- Review loop mechanics, the Merge Gate, and `scripts/pr_review.py`: pr-review-conduct. +- Branch rules, never delete develop, the EOL-only conflict, issue-closing keywords belonging on + the promotion PR: operational-vs-release-workflow. +- Worktree isolation and post-merge cleanup: repo-worktree. + +## Stop and Ask, Beyond the How-Far Question + +- A genuine design trade-off, a recurring finding pattern, or an architectural redesign proposal + each escalate per pr-review-conduct's own list, restated there, not duplicated here. +- An unrecognized review shape blocks the gate on its own, file an issue naming it and ask, never + guess what new wording probably meant. diff --git a/.github/skills/fleet-conformance-check/SKILL.md b/.github/skills/fleet-conformance-check/SKILL.md new file mode 100644 index 0000000..2b6e840 --- /dev/null +++ b/.github/skills/fleet-conformance-check/SKILL.md @@ -0,0 +1,74 @@ +--- +name: fleet-conformance-check +description: >- + Checks, from inside a downstream repo's own session, whether this repo and this machine are + current against the ptr727/ProjectTemplate hub, and safely self-applies what it can. Use this + whenever asked to check if this repo is up to date with the hub, whenever a fleet rule or Skill + seems to not be applying and the cause is unclear, or whenever about to work in a fleet repo and + wanting to confirm the ground under that work is current before trusting it. Needs no standing + hub checkout of its own and no named target repo, only the repo the session is already in, + though the check itself fetches a hub checkout to reach scripts/skills_install.py, since + scripts/ is hub-hosted rather than carried. This is the counterpart to resync-a-repo, which + needs both a hub checkout already in hand and a named external target to drive change from the + hub side instead. Also triggers on "why do I have to keep restating this rule every session," + since a stale or missing Skills install is the most common cause and the cheapest one to rule + out first. +--- + +# Fleet Conformance Check + +## Why this exists + +A downstream repo today only finds out it has drifted when someone runs a hub-driven resync +against it by name. Nothing notices from the inside on its own. This skill is that inside check, +run with no hub-side operator watching, so a stale Skills install or an out-of-date `AGENTS.md` +pointer gets noticed and fixed without waiting for a fleet-wide sweep to reach this particular +repo. + +## What it checks + +1. **Is the Skills install current on this machine.** `scripts/` is hub-hosted and reached rather + than carried, per GOVERNANCE.md "Hub-Hosted Tooling", so fetch a hub checkout + (`github.com/ptr727/ProjectTemplate`, `main` branch, fetched fresh) and run + `python3 scripts/skills_install.py --report` from it. A stale or missing stamp is very often + the direct answer to "why isn't a fleet rule applying": the harness never loaded the current + content in the first place, and no amount of re-reading `GOVERNANCE.md` fixes that. +2. **Does this repo's own carried content still match the hub.** Compare `AGENTS.md`'s + "Where the Rules Live" pointer text, and any other verbatim `AGENTS.md`/`GOVERNANCE.md` section + this repo carries, against the same hub checkout's current wording, by reading the text rather + than by feel. + +## What it is safe to fix on its own + +- **Re-run the installer**, `python3 scripts/skills_install.py`, when the stamp reports stale. + This is a per-machine, local-only change, nothing in it touches this repo's git history or + needs a review. + +Nothing else. This skill never re-vendors a carried file, never deletes one, and never applies a +setting or ruleset. Those are `resync-a-repo`'s job, driven from the hub with a named target, +never a downstream repo acting on itself. + +## Refresh cadence + +Re-run the installer when `--report` exits non-zero, and after any hub merge that touches +`.agents/skills/`. Session entry runs no automatic check, by design: the trigger is suspicion, +and the restated-rule symptom below is the loudest form of it. `docs/host-setup.md` +"Fleet Skills Install" in the hub states the same cadence for the host side, and an automated +refresh stays out of scope until the fleet has evidence the manual cadence fails. + +## What it escalates instead of touching + +- **A carried section that differs from the hub in a way that reads as a genuine local addition** + rather than plain staleness, the exact case `carried-instruction-file-guard` exists to protect. + Report precisely what differs and stop there. Per AUDIT.md, a downstream repo does not write its + own audit report or resync itself against the hub, it names what it found and points at + `resync-a-repo`, run from a hub checkout, as the next step. +- **Anything the installer alone cannot resolve**, a broken `claude` CLI marketplace + registration, a settings or ruleset drift, a workflow interface mismatch. Name it and hand it to + the maintainer or a hub-driven resync rather than patching around it locally. + +## Answering "why isn't a fleet rule applying" + +Check the install stamp first, before assuming a Skill's description is worded wrong or that the +rule was never carried to this repo at all. It is the most common cause, and it is the cheapest +one to confirm. diff --git a/.github/skills/git-commit-conventions/SKILL.md b/.github/skills/git-commit-conventions/SKILL.md new file mode 100644 index 0000000..5c1454f --- /dev/null +++ b/.github/skills/git-commit-conventions/SKILL.md @@ -0,0 +1,167 @@ +--- +name: git-commit-conventions +description: >- + Governs how an agent stages, commits, signs, and pushes in a ptr727/ProjectTemplate fleet repo: + default-to-staging vs. explicit commit authorization, why "commit" means commit-and-push, the + mandatory signed-commit and noreply-identity checks, never force-pushing, how a history rewrite + must re-identify a commit that is not the agent's own, and the destructive-git-command ban. Use + this whenever about to run git add/commit/push, whenever authorization to commit is ambiguous + ("fix this" versus "commit this"), whenever about to configure or verify commit signing or + git user.email, whenever a merge conflict or a stale branch tempts a force-push or a hard reset, + and whenever rewriting history (filter-repo, an interactive rebase equivalent) touches a commit + authored or committed by someone else. Triggers even when the task looks like routine + housekeeping, such as "clean up this branch" or "just push it", because a scope-widened commit + authorization, an unsigned commit, a fabricated identity, or a force-push are each easy to do by + habit and each one is a hard-to-reverse mistake on a shared branch. +--- + +# Git Commit Conventions + +## Why this exists + +These are the fleet's mechanical git rules for producing a commit, kept in one place instead of +re-derived per repo or per session: whether to commit at all, what committing implies, how +signing and identity are verified rather than configured, and which commands are never run +without being asked. None of these are style preferences. Branch protection enforces several of +them at push time, and the rest guard against damage a rejected push does not undo (a +scope-widened commit, a rewritten shared history, a destructive reset). + +## Staging versus committing + +- **Default to staging, not committing.** Stage with `git add` and leave `git commit` to the + developer unless the developer has explicitly authorized committing for the current ask ("commit + this", "open a PR"). Authorization is scope-bound: it covers the commits that specific task + needs, not a blanket license for the rest of the session. +- **Stage by explicit path, never `git add -A` or `git add .`.** A blanket add stages whatever + else happens to be in the tree, and what it sweeps in is another task's uncommitted work, + landing in a commit whose subject never mentions it, committed by a session that never saw it. + That sweep has happened, which is why task isolation exists (the `repo-worktree` skill), and + isolation makes a shared tree rare rather than impossible. Name the files this task changed, + and let anything else stay unstaged. +- **"Commit" means commit and push.** An authorization to commit carries the push to the feature + branch the work belongs on, because nothing reviews a local commit. The Copilot review loop, the + required status checks, and the maintainer all read the remote, so stopping at `git commit` + leaves the review unstarted and the branch's state private to one machine, which reads as + progress while none of the gates have run. Push to the feature branch, never to a protected + branch, and never with `--force`. Holding a commit locally is the narrower case: it happens when + the developer asks for it, not by default. +- **Check `git status` before committing, and treat any change this session did not make as a + stop.** The maintainer hand-edits files live, often `README.md`/`HISTORY.md`, sometimes with an + editor's LF -> CRLF flip on top, and a sibling agent session sharing the tree leaves its edits + the same way. Whoever the author is, a change this session did not make is never bundled: ask + whether to include it, or leave it unstaged and say so, rather than committing half-finished + work or stranding it in an unrelated commit. An unexpected change in the tree is also the + signal to re-check isolation per the `repo-worktree` skill, since it may mean another task is + live in this checkout. + +## Signing, verified not configured + +- **Every commit must be cryptographically signed (SSH or GPG).** Branch protection enforces this + on every fleet branch, and an unsigned commit is rejected on push. Signing depends on + environment configuration (`commit.gpgsign`, `user.signingkey`, `gpg.format`), but none of those + values prove signing actually works: `gpg.format=ssh` can sign straight from a key file with no + `ssh-agent` running at all (the common case on Git for Windows), just as GPG can sign + agent-backed or straight from a keyring. **Probing agent liveness (`ssh-add -L`, a `gpg-agent` + check) is not a valid test and must not be used.** It tests one specific delivery path, not + whether a commit actually ends up signed, and a host that signs straight from a key file fails + that probe while signing correctly. +- **Verify with a real scratch commit, read back with git's own verdict, not a text grep.** This + single probe is tech-agnostic (SSH agent-backed, SSH key-file, GPG agent-backed, and GPG keyring + all exercise the same code path) and doubles as the identity check below. Run it once before the + first agent-authored commit of a session. Don't assume a prior session left config correct. The + commit below is plain, deliberately no `-S`: forcing it would still succeed on a host where + `commit.gpgsign` is unset or false, which is the exact default-config gap this probe exists to + catch, since every real commit an agent makes is plain too: + + The probe is one physical line, not backslash-joined ones, so it copy-pastes cleanly into a + shell: + + ```sh + d=$(mktemp -d "${TMPDIR:-/tmp}/sign-check.XXXXXX") && ( trap 'rm -rf "$d"' 0; email=$(git config --global --get user.email) && git init -q "$d" && git -C "$d" commit --allow-empty -q -m check && out=$(git -C "$d" log -1 --format='sig=%G? author=%an <%ae> committer=%cn <%ce>') && echo "$out" && ae=$(git -C "$d" log -1 --format='%ae') && ce=$(git -C "$d" log -1 --format='%ce') && case "$out" in sig=G\ *|sig=U\ *) true ;; *) false ;; esac && case "$email" in *@users.noreply.github.com) true ;; *) false ;; esac && [ "$ae" = "$email" ] && [ "$ce" = "$email" ] ) + ``` + + PowerShell equivalent: + + ```powershell + $d = Join-Path $env:TEMP ([guid]::NewGuid()) + try { + $email = git config --global --get user.email + git init -q "$d" ` + && git -C "$d" commit --allow-empty -q -m check + $out = git -C "$d" log -1 --format='sig=%G? author=%an <%ae> committer=%cn <%ce>' + $out + $ae = git -C "$d" log -1 --format='%ae' + $ce = git -C "$d" log -1 --format='%ce' + if ($out -notmatch '^sig=[GU] ' -or $email -notmatch '@users\.noreply\.github\.com$' ` + -or $ae -ne $email -or $ce -ne $email) { + throw "signing/identity check failed: $out" + } + } finally { + if (Test-Path "$d") { Remove-Item -Recurse -Force "$d" } + } + ``` + + `sig` must read `G` (good signature) or `U` (good signature, unrecognized signer). For GPG, `U` + is a valid signature from a key whose trust level is merely undefined, common right after + generating a new key. For SSH, it's a valid signature from a key not found in the local + `allowed_signers` file, which doesn't affect whether GitHub itself verifies the commit, only + local `git verify-commit` output. `sig` is git's own verdict char. Don't grep localized + "Good" text, since that varies by git version and locale. Anything else, or the commit failing + outright, means **do not commit**: surface the actual error to the developer and stop at + `git add`. Nothing else is contrary evidence: not an unreachable agent, not a config value, not a + signature type you can't otherwise explain in past history (see below). +- **A mix of SSH- and GPG-signed commits in history is structural, not a host to track down.** + `git log --pretty='%G? %GK'` shows two distinct shapes, not two health states: a commit committed + by the PR's own author carries that host's own signature type, while a commit committed by + `GitHub <noreply@github.com>` is a squash-merge: GitHub creates and signs that commit itself, + server-side, with GitHub's own GPG key, regardless of what the PR author signed with locally. + Every commit on `develop`/`main` past its first squash-merge shows `GitHub` as committer and a + GPG signature. That's expected on every fleet repo, on every host, and is not evidence anything + is misconfigured. Check `commit.committer.name` before treating a differing signature type as a + clue worth chasing. +- **Signing must be live before the *first* commit, not retrofitted.** Turning on a + require-signed-commits rule against a branch that already carries unsigned commits forces a + rewrite of that entire history to re-sign it, changing every commit SHA and making whoever does + the rewrite the committer and signer of every commit in it (a rebase preserves `author` but not + the original signatures, and one contributor cannot sign for another). During new-repo setup, + never create commits until signing is verified. + +## Identity, verified not set + +**Commit under the committing account's own GitHub `noreply` identity, never a private, personal, +or invented address.** `author` and `committer` on every agent-authored commit are the GitHub +`noreply` address of the account whose key signs the commit, in `username@users.noreply.github.com` +or `ID+username@users.noreply.github.com` form. **Verify it, do not set it**: the scratch commit +from the signing check above already proves this end-to-end. Read its `author=`/`committer=` +output rather than trusting `git config --get user.email` alone, since a global config value +doesn't prove what actually lands on a commit object, and read both rather than the author alone +since a rebase, amend, or cherry-pick can rewrite the committer while leaving the author +untouched. Match both against that address before committing, rather than +writing a repo-local override. The identity is host configuration set globally once, so a repo-local +`user.email` is redundant where the global is right and a silently-shadowing wrong identity where +it is not. A mismatch is a host fault to surface to the maintainer, not to patch per repo, because +a local override hides a broken host that then commits wrong in every other repo on that machine. +A wrong identity is not cosmetic: a private email trips GitHub's email-privacy push protection, and +an invented author pollutes history. It is also a distinct failure from signing (a wrong author +does not by itself fail the signature check), though the ad-hoc identities that produce one are +typically also unsigned, which the signing rule above then rejects independently. + +## Never force push + +Do not run `git push --force` or `git push --force-with-lease` under any circumstances. Force +pushing rewrites shared history and can cause data loss. This holds regardless of how confident +the rewrite looks, a rejected push is recoverable, a force-pushed one is not. + +## History rewrites re-identify only what changed + +**Do not rewrite a commit that does not need to change.** A history rewrite (e.g. `git filter-repo` +to strip PII) re-signs every touched commit with the rewriter's key. If that commit is still +committed by a bot (`dependabot[bot]`, `github-actions[bot]`) or GitHub's own web-flow, the +signature will not match the committer and the require-signed-commits rule rejects it. Scope the +rewrite to only the commits that must change. Set `committer` (and `author`) to the rewriter's +identity on any non-own commit that must be modified. Verify with `git log --show-signature` after +any rewrite. See `references/history-rewrite.md` for the full two-gate rule. + +## Never run destructive git commands without being asked + +`git reset --hard`, `git checkout .`, `git restore .`, `git clean -f`, and other commands that discard work require explicit developer instruction. Never use them as a convenience inside a larger task. One narrow cleanup exception applies to `git branch -D <exact-task-branch>` after a squash merge. It requires live proof that the pull request for that exact branch merged and a clean worktree at the verified head SHA, per `repo-worktree`. The exception never applies to `develop`, an unmerged branch, an unresolved pull request, or a branch with uncommitted work. diff --git a/.github/skills/git-commit-conventions/references/history-rewrite.md b/.github/skills/git-commit-conventions/references/history-rewrite.md new file mode 100644 index 0000000..b331a55 --- /dev/null +++ b/.github/skills/git-commit-conventions/references/history-rewrite.md @@ -0,0 +1,24 @@ +# History Rewrites: Re-identification Rules + +**A history rewrite includes only the commits that must change, and re-identifies any commit it +rewrites that is not the agent's own.** Filtering history (`git filter-repo` or an equivalent, for +example to strip PII) re-signs every commit it touches with the rewriter's own key, while the +tooling preserves each commit's original `author`/`committer` unless told otherwise. GitHub +verifies a signature against the commit's `committer` identity, so a signature from the rewriter's +key over a commit still committed by a bot (`dependabot[bot]`, `github-actions[bot]`) or GitHub's +own web-flow does not match its committer and lands `unknown_key`/unverified, which a +require-signed-commits rule then rejects. + +Two gates keep committer and signature aligned: + +1. **Scope the rewrite to only the commits that must be modified.** By default those are the + rewriter's own, whose committer already matches, so a commit that needs no change stays out of + the rewrite entirely and its identity and signature are never touched. +2. **If a commit that must change is not the rewriter's own, set its `committer` to the rewriter's + own signing identity before re-signing** (and its `author` too, since a rewrite that alters + content should not keep attributing it to the bot). The original bot attribution is deliberately + given up as the cost of having to rewrite it. + +Never leave a signature over a commit committed by another identity. Verify after any rewrite that +every rewritten commit is signed and committed under the correct identity +(`git log --show-signature`). diff --git a/.github/skills/local-strict-review/SKILL.md b/.github/skills/local-strict-review/SKILL.md new file mode 100644 index 0000000..cad9a06 --- /dev/null +++ b/.github/skills/local-strict-review/SKILL.md @@ -0,0 +1,71 @@ +--- +name: local-strict-review +description: >- + Runs one read-only, adversarial review pass against this branch's current diff against its + target branch, full file context included, on the strongest model tier the session can reach, + before a unit of work is pushed toward a pull request or claimed done. Use this whenever staged, + committed, or untracked work is about to be pushed on a PR-bound branch, and whenever + `agent-conduct`'s "about to claim work is done, verified, green, or fixed" trigger fires for + PR-bound work. Triggers even when the change looks small or the same session already judged its + own diff ready, because a self-review pass judging its own diff inherits its own blind spots, + the exact gap this skill exists to close before a PR-hosted reviewer closes it instead. Reuses + `code-review`'s "Review the Change" criteria rather than restating them, and owns only this + local, pre-PR moment. Once a pull request exists, `pr-review-conduct` and `drive-pr` own + triaging and disposing of what a PR-hosted reviewer finds. +--- + +# Local Strict Review + +## Why This Exists + +A coding agent that finishes a unit of work, judges it ready, and opens the pull request is judging its own diff with the model, and often the blind spots, that wrote it. CodeRabbit, Qodo, and Copilot routinely find real defects that a local pass missed, and each round costs review latency and, for a rate-limited reviewer, shared account-wide quota. A local, full-file-context adversarial pass before the pull request exists catches the same class of defect for a fixed, smaller cost, the same reasoning that already runs local lint before a push instead of waiting for CI. + +## What It Does + +Dispatches one read-only subagent against this branch's full diff since it forked from its target branch. Resolve `<target>` once, `develop` unless `repo-worktree`'s base-branch rule put this branch on `main` instead, then fetch it, `git fetch origin <target>`, and diff against the merge-base, `git diff "$(git merge-base origin/<target> HEAD)"`. Stop and report a failed fetch rather than running the merge-base or diff commands anyway: an existing local `origin/<target>` ref can still resolve after a failed fetch, and reviewing against it silently trades the current target for a stale one. Use the same resolved `<target>` in every command below, never a literal `develop` alongside it. Naming the target branch explicitly matters: the branch's own `@{u}` tracking ref points at the branch's own remote once it has been pushed, not at the branch it targets, so anchoring there silently narrows a later run to only the diff since the last push instead of the full accumulated diff. That merge-base diff covers every commit already on the branch plus whatever is currently staged or unstaged, so it is never empty and never reviews only the latest increment, at any of the moments this skill is invoked from. A fresh review of the full accumulated diff is what catches what per-push review misses, the exact evidence this skill exists to act on. + +`git diff` never reports a path `git add` has not touched, so a newly created file sitting untracked would otherwise go unread. List it explicitly, `git ls-files --others --exclude-standard`, and read each result in full alongside the diff, the same as any other file the diff touches. + +The subagent reads the full content of every file the diff and the untracked-file list touch, not just the hunks, since cross-file and whole-file context is exactly what incremental review misses. It reports findings only. It never fixes, stages, or commits anything. + +Review criteria are `code-review`'s "Review the Change" section, reused rather than restated here, plus three traps worth calling out explicitly for a pass that runs before a human or a PR-hosted reviewer ever sees the diff: unguarded type coercions, TOCTOU/race conditions, and platform-specific behavior differences. `code-review`'s separate "Publish Every Finding" section does not apply here: this skill has no PR to post a comment on and no coverage marker to close a review with, so its own report contract below replaces that section rather than extending it. + +## Running It + +Follow `AGENTS.md` "Context and Delegation Discipline"'s subagent briefing shape: + +```text +Task: adversarial review of this branch's diff against its merge-base with its target branch, + read full surrounding files where the diff hunks alone do not give enough context. +Paths: the files `git diff --name-only "$(git merge-base origin/<target> HEAD)"` and + `git ls-files --others --exclude-standard` list, mandatory floor. Reading a specific + unchanged caller or consumer beyond that list is in bounds only where a candidate finding's + proof actually depends on it, per code-review's own "follow data and control flow beyond the + edited lines" instruction below, never as an open-ended exploration. +Rules that bind this task: quote `code-review`'s "Review the Change" section into the prompt, + plus flag unguarded type coercions, TOCTOU/race conditions, and platform-specific behavior + differences explicitly. Do not quote "Publish Every Finding", this task's report contract is + the Return line below, not a PR comment or a coverage marker. +Return: one finding per line, file:line, the concrete failure scenario, no severity theater. +Bounds: read-only. No edit, no stage, no commit, no push, no PR-hosted write of any kind. +<AGENTS.md's own unresolved-rule closing line, quoted verbatim from "Context and Delegation Discipline", not restated here> +``` + +**Model tier:** the strongest tier this session can reach, per `AGENTS.md` "Match the model tier to the judgment" and "Never tier down the seat holding the judgment", applied here to the reviewer rather than the author. Run the pass on the same tier that authored the change when only one tier is reachable, a second, adversarially-prompted look still catches what the authoring pass's own "looks ready" judgment did not. + +## Disposing of Findings + +Every finding maps to one of `pr-review-conduct`'s five outcomes before the pull request opens: fixed, evidence-disproven, filed as a deferred issue, escalated to the maintainer for an explicit call, or, if it keeps recurring, taken as a signal to fix the class. A finding this pass raised and not fixed is never the agent's own call to just leave. Per outcome 3, that decision needs the maintainer's explicit answer, the same way a PR-hosted finding would. Running this pass is expected before every push toward a pull request, per `agent-conduct`. Its findings stay advisory: a finding it raises does not by itself block `git commit` or `gh pr create`, the disposition above is what closes it, the same posture local lint holds today. It posts nothing to GitHub, it only reports to the session driving the work. A finding raised here and not fixed is not thereby resolved: the same finding shape reaching a PR-hosted reviewer later still gets its own fresh disposition, per `pr-review-conduct`'s "a disposition decided on one PR does not carry to the next." + +## When to Run It + +- Before the first push toward a pull request (`drive-pr`'s Drive Loop step 2, `pr-review-conduct`'s Expected review loop step 1). +- Before pushing a fix for a reviewer finding, the same self-review blind spot applies to a fix as to the original diff (`drive-pr`'s "Disposing of Every Finding", `pr-review-conduct`'s outcome 1). +- Whenever `agent-conduct`'s "about to claim work is done, verified, green, or fixed" trigger fires for work that will become, or already is, a pull request. + +## Mechanics Live Elsewhere + +- Review criteria: `code-review`. +- Delegation shape and model-tier discipline: `AGENTS.md` "Context and Delegation Discipline". +- Branch base rule (`develop` unless the task is explicitly `main`-only): `repo-worktree`. +- Finding disposition once a pull request exists, the Merge Gate, `scripts/pr_review.py`: `pr-review-conduct`, `drive-pr`. diff --git a/.github/skills/merge-and-release/SKILL.md b/.github/skills/merge-and-release/SKILL.md new file mode 100644 index 0000000..3eb31c3 --- /dev/null +++ b/.github/skills/merge-and-release/SKILL.md @@ -0,0 +1,225 @@ +--- +name: merge-and-release +description: >- + Merges a ready develop -> main promotion PR for any ptr727/ProjectTemplate fleet repo and, when + asked, dispatches the release, in this hub always refreshing this machine's installed Skills + from the newly promoted content as part of that release step, never as a separate ask. Use this + whenever asked to merge main, ship a release, cut a release, or finish a promotion once its PR + is already green and fully resolved (produced by drive-pr or by hand). When the request does + not say how far ("merge main", "ship it"), ask once whether to merge only or merge and release, + rather than guessing which the maintainer wants this time. Triggers even when the phrasing is + as short as "merge main and release", because that already states the scope and is itself the + explicit, current go-ahead this skill acts on without asking again, though it never substitutes + for the pr-review-conduct Merge Gate, a promotion PR that is not actually green and fully + resolved gets reported and stopped on, not merged. +--- + +# Merge and Release + +## Why This Exists + +Once drive-pr (or a maintainer by hand) leaves a promotion PR ready, the same two steps follow +every time: merge it, and usually dispatch the release it unblocks. In this hub a promotion can +also change `.agents/skills` content this very session depends on, so the release step always +carries a Skills refresh with it there, never a separate branch to ask about, an ambiguous "merge +and release" on the hub must not leave the maintainer unsure whether Skills got refreshed. One +skill covers all of it, scoped down by what the maintainer actually asks for. + +## How Far to Go + +- Read the invocation for an explicit scope first. "Just merge" or "merge only" means stop after + the merge. "Merge and release", "ship it", or "cut a release" means also dispatch, and in this + hub also refresh Skills as part of that same step. Act on either without asking. +- When the request names no scope ("merge main"), ask once, before merging: merge only, or merge + and release. Recommend "merge and release" as the default on a release-model repo, a promotion + merged without its release is the more common regret there. Recommend "merge only" as the + default on an operational repo (registry `workflowModel: operational`), where a release is a + separate, deliberate dispatch rather than an automatic follow-on to a promotion, per + operational-vs-release-workflow's "Operational repositories" delta. +- Detect the hub automatically, `git remote get-url origin` or `gh repo view --json + nameWithOwner` naming `ptr727/ProjectTemplate`. There the release scope silently includes the + Skills refresh, a downstream repo never sees it, it has no `.agents/skills` of its own to + refresh. + +## What Invoking This Skill Authorizes + +- Naming this skill, and answering its how-far question, is the maintainer's explicit, current + go-ahead to merge the promotion PR and to perform the scope chosen, for the one repo and PR in + front of the agent. It is never a standing mode carried to the next PR. +- It is never permission to merge a PR that fails the Merge Gate. Re-verify the gate at + invocation time, a check from earlier in the session can be stale. + +## The Procedure + +1. Identify the open develop -> main promotion PR for this repo, stop and report if none is open. +2. From a hub checkout, `scripts/` is not carried into downstream repos, run `scripts/pr_review.py + status [number] --repo owner/repo` on it and confirm the pr-review-conduct Merge Gate. Stop + and report exactly what is missing rather than merging on a partial gate. +3. `gh pr merge [number] --merge --repo owner/repo`. Never `--delete-branch`, the promotion PR's + head is `develop`. +4. Confirm the merge landed, `mergedAt` set, `main`'s tip matching the merge commit. +5. When the chosen scope includes a release, first bring the hub checkout used for this procedure + current, `git fetch origin main`, and read this repo's `releaseTrigger` from that fetched tip + rather than a possibly-stale working tree copy, relevant when the target repo is the hub itself + and this exact promotion changed its own registry entry. Select the one matching entry + explicitly, falling back to the registry's own default when that entry sets no + `releaseTrigger` of its own, and stop and report rather than guessing when selection is not + exactly one match, on a non-1 count exit non-zero rather than returning empty with success, an + ambiguous or missing match must fail loud, not read as an empty value still safe to act on: `git + show origin/main:registry/repos.json | jq -r --arg name '<repo-name>' '(.repos | map(select(.name + == $name))) as $m | if ($m | length) == 1 then ($m[0].releaseTrigger // .defaults.releaseTrigger) + else error("expected exactly one registry entry for \($name), got \($m | length)") end'`. Two + cases, `none` versus anything else. When it + reads `none`, report that no + release is configured, dispatch and run-correlation (step 6) do not apply. Otherwise (`two-phase`, + `dispatch-only`, or `publish-on-merge` alike), dispatch explicitly, `gh workflow run + publish-release.yml --ref main --repo owner/repo`, or `--ref develop` only when the maintainer + explicitly asked for a prerelease dispatch instead. `publish-on-merge`'s automatic publish is + gated on the actor being the codegen App merging a Dependabot or codegen PR + (operational-vs-release-workflow's publishing rules), so an ordinary human promotion merge, + exactly what step 3 just did, never triggers it, this step's explicit dispatch is what actually + ships the release here, not a side effect of the merge. +6. Correlate the specific run this dispatch produced rather than assuming the newest one is it. + `gh run list --repo owner/repo --workflow publish-release.yml --branch main --event + workflow_dispatch --json databaseId,createdAt,headSha` (or `--branch develop` for a prerelease + dispatch), matched by `headSha` against the dispatched ref's tip (`main`'s tip confirmed in + step 4, or `develop`'s current tip for a prerelease) and by `createdAt` against the dispatch + time. `gh run list` can momentarily omit a just-created run, so a single query reporting zero + candidates is not yet "never started". Poll the list itself, within a bounded interval, until + exactly one candidate matches. A concurrent run of a different event on the same branch must + never be mistaken for this one, more than one candidate is as inconclusive as zero. A run whose + `headSha` does not match the expected tip at all, rather than simply being absent, means the + dispatched ref moved between step 4's confirmation and the dispatch itself, report that + distinctly, the ref changed mid-dispatch, rather than folding it into an ordinary absent-run + timeout. Report and stop rather than guessing once the interval elapses with zero or more than + one candidate still matching. Only once exactly one candidate is confirmed, poll that one run + id to completion in one further bounded background wait with an explicit, finite timeout, + 2700 seconds (45 minutes, matching `scripts/pr_review.py`'s own default) unless the maintainer + states a different bound for this specific release: `timeout 2700 gh run watch <run-id> --repo + owner/repo --exit-status` on a host with GNU `timeout`, or the equivalent bounded-wait + mechanism enforcing the same bound on a host without it (macOS without coreutils, native + Windows). Report a timeout separately from a completed run's own conclusion, the tag or + version it produced. A run that fails, times out, or never starts is reported, never silently + retried. +7. In the hub, when the chosen scope includes a release, bring this checkout to the merged + content without discarding or mixing in anything local. First assert `git status --porcelain + --untracked-files=all --ignored -- .agents/skills/ .claude-plugin/` is empty, and stop and + report rather than proceeding over any uncommitted content there, tracked, untracked, or + gitignored, since `skills_install.py` reads both paths: `shutil.copytree()` installs each + `.agents/skills/` skill directory for Codex/opencode, and `claude plugin marketplace add` + installs from `.claude-plugin/` for Claude Code, so a gitignored stray file under either rides + along the same as any other, and the plain porcelain form (silent on ignored paths) would pass + this preflight while one still rides into an install. Scoped to those two paths rather than + the whole tree, matching `skills_install.py`'s own `source_ref()` dirty check (`watched = + [SKILLS_SRC, CLAUDE_PLUGIN_DIR]`), since an ignored file elsewhere in the checkout (a build + cache, a lockfile) is not this preflight's concern and should not block the refresh on it. + Then `git fetch origin main`, `git checkout main` + (or `git checkout -b main origin/main` the first time this checkout carries no local `main` at + all, `checkout` rather than `switch` since the fleet's own `git` floor is undeclared and + `checkout` needs no minimum version for this), and `git merge --ff-only origin/main`. + `checkout` still refuses a `main` checked out in another worktree, and `--ff-only` refuses + anything but a clean fast-forward, so either stops and reports on top of what the preflight + already ruled out, per Repository Boundaries and Write Safety. `--ff-only` does not fail when + local `main` is already ahead of `origin/main`, since a strict superset needs no fast-forward + and reports up to date, so assert `git rev-parse main` equals `git rev-parse origin/main` + afterward and stop and report on a mismatch, a local-only commit this checkout never pushed is + exactly the case a bare "up to date" would hide. `skills_install.py` stamps and installs from + whatever this checkout's HEAD already is, so running it against a stale, unrefreshed, or + locally-diverged `main` skips the refresh silently. Only then run `python3 scripts/skills_install.py --report`, then + `python3 scripts/skills_install.py` to install, and confirm `--report` now reads current, + regardless of whether step 5 or 6 dispatched, skipped, or failed a release, this step is gated + only on the chosen scope, never on the release outcome. This refreshes only the machine running + this session, per skill-lifecycle, every other machine still refreshes on its own next run or + `docs/host-setup.md` "Fleet Skills Install" cadence. +8. Run cleanup regardless of how steps 5 through 7 ended, no release configured, a dispatch + failure, an ambiguous run match, a timeout, a failed run, or a hub Skills refresh all still + reach this step, the merge in step 3 already landed by then. Two parts, both required, neither + optional: + - The promotion PR's own worktree: fetch and prune, remove the worktree, then fast-forward the + base clone to `develop`. Removing first, not after, matters: the base clone cannot check out + `develop` while the promotion worktree still has it checked out, one branch checked out in + two worktrees at once is refused outright. Never delete `develop`, it is the promotion PR's + own head, and the repo's auto-delete-head-branches setting is kept off fleet-wide for exactly + this reason, so nothing does this automatically. + - A defensive sweep for anything drive-pr's own cleanup should already have removed but might + not have, an interrupted loop, a fix landed by hand outside that skill, or a maintainer + merge in the GitHub UI. `git worktree list` for any worktree still registered under this + task's feature branches, `git branch -vv` for any local feature branch, `git ls-remote + --heads origin` for any matching remote feature branch. For each, verify it finished by + reading GitHub's own state with the exact fields this check needs, not a bare listing, and + stop and report rather than guessing when selection is not exactly one match, on a non-1 + count exit non-zero rather than returning empty with success, an ambiguous or missing match + must fail loud, not read as an empty value still safe to act on: `gh pr list --head + "<branch>" --state merged --repo owner/repo --json + number,baseRefName,mergedAt,headRefOid,headRefName,headRepository --jq 'if length == 1 then + .[0] else error("expected exactly one merged PR for this head, got \(length)") end'`. + `--head` is expected to match exactly (verified against `gh` 2.97.0 on this repo, a bare + prefix of a real branch name returned nothing), but confirming `headRefName` equals `<branch>` + costs one field and is cheap insurance against a future `gh` behavior change, not a workaround + for a known partial-match case. Confirm `headRepository` is non-null and its `nameWithOwner` equals + `owner/repo`, the owner alone is not enough, a same-owner PR against an identically named + branch in a different repository must never pass this check either. Confirm `baseRefName` + is `develop` (a different merged pull request can share the same head branch name against a + different base, and that is never this sweep's target) and `mergedAt` is set. Compare tips + only where a remote branch actually exists. `git ls-remote --heads --exit-code -- origin + "refs/heads/<branch>"` is the exact-match form and must be, in that argument order. `--heads + origin "<branch>"` alone still tail-matches, a bare `topic/x` pattern also returns an unrelated + `other/topic/x` if one exists. `--` placed after `origin` instead of before it is not + equivalent either, verified empirically: with a `refs/heads/other/--` ref present, `--heads + origin -- "refs/heads/<branch>"` matched both that ref and the intended one, while `--heads -- + origin "refs/heads/<branch>"` matched only the one intended. Exit status is a tri-state, not a + stdin-emptiness check: `--exit-code` makes exit `2` mean query succeeded, branch gone, most + likely a prior cleanup attempt got interrupted after the remote delete but before the local + one, so skip straight to the local-tip check below and never attempt the remote delete a + second time. Exit `0` means it matched. Anything else is a failed query, a network or auth + problem, and stops and reports rather than being read as absence, an unreachable remote and a + genuinely gone branch both print nothing to stdout, only the exit code tells them apart. + Where the remote branch does exist, its tip must match that exact pull request's `headRefOid` + before its own delete proceeds, proving nothing landed on it since. Where a local branch + still exists too, its tip (`git rev-parse --verify "refs/heads/<branch>"`) must independently + match `headRefOid` before its own delete proceeds. Neither side needs the other to exist, a + prior interrupted attempt may have deleted one side already and left only the other, so + verify and delete whichever side is still there and skip whichever already is not, never + block one side's cleanup on the other side's absence. No `--` on `rev-parse`, verified + empirically: `git rev-parse -- "<branch>"` treats the argument after `--` as a path rather + than a revision and never resolves a SHA at all. The fully-qualified form needs no `--` + regardless, since `refs/heads/<branch>` never itself starts with `-`, and `--verify` fails + loudly rather than guessing when it does not resolve. Every branch or + worktree-path placeholder below is the real value, substituted as its own quoted argument + (a shell variable expansion such as `"$branch"`, or an argv element), never handed to `eval` + or `sh -c` for a second round of shell parsing, the only way an embedded `$()` or backtick + would actually run. A valid ref can start with `-` or carry a shell metacharacter, which is + why it stays quoted regardless. `--` marks the + end of options wherever a command supports it. + `git merge-base --is-ancestor <branch> develop` must never be used for either tip check, a + squash merge (drive-pr's own merge method) never makes the feature tip a literal ancestor of + `develop`, so the check reports every already-finished branch as unmerged. Only once GitHub + confirms it, and only when a local worktree or branch is still there to remove, remove the + worktree by its exact path (a dirty worktree stops cleanup rather than discarding uncommitted + work), `git worktree remove "<worktree-path>"`, `git worktree list` names it, then delete the + local branch. `git branch + -d` has the identical squash blindness as `git merge-base --is-ancestor` and refuses too, so + use `git branch -D -- "<exact-branch>"` here, safe only because the GitHub-state check just + proved that exact branch finished, the narrow post-squash exception git-commit-conventions + describes, never applied to an unverified branch. Then, only when the remote branch still + exists, delete it the same way, `git push origin --delete -- "<branch>"`. + Never `--force-with-lease` here, git-commit-conventions + forbids it unconditionally, the GitHub-state check just completed is the verification gate, + not a compare-and-swap at delete time. Never apply this sweep to `develop` or `main` + themselves, only to feature branches a drive-pr loop created. + +## Mechanics Live Elsewhere + +- The Merge Gate itself: pr-review-conduct. +- Never delete develop, no-op republish, the operational repos' dispatch-only model: + operational-vs-release-workflow. +- What the dispatch actually builds and publishes: workflow-ci-contract. +- Skills install and report semantics: skill-lifecycle. +- Cleanup mechanics: repo-worktree. + +## Stop and Report, Never Guess + +- A merge conflict, a newly failing check, or a gate item that regressed since drive-pr finished + are each a stop, report the exact state, never force or retry blindly. +- `gh pr merge` or `gh workflow run` failing is reported with its actual output, never + suppressed, never assumed harmless on the agent's side alone. diff --git a/.github/skills/operational-vs-release-workflow/SKILL.md b/.github/skills/operational-vs-release-workflow/SKILL.md new file mode 100644 index 0000000..9a9e9f2 --- /dev/null +++ b/.github/skills/operational-vs-release-workflow/SKILL.md @@ -0,0 +1,156 @@ +--- +name: operational-vs-release-workflow +description: >- + Governs how a ptr727/ProjectTemplate fleet repo branches, promotes, and publishes: the + feature -> develop -> main flow, squash-only vs. merge-commit-only branch protection, the two + develop -> main promotion traps (never delete develop, EOL-only conflicts), the two-phase + publish model (PRs smoke-test only, a human merge never auto-publishes), NBGV semantic + versioning, and the operational-repo delta (direct-to-develop commits, advisory CI, dispatch-only + release) that applies instead whenever the registry's workflowModel field for this repo reads + operational rather than release. Use this whenever choosing a target branch for a change, + promoting develop to main, resolving a develop -> main merge conflict, deciding whether a + release repo's config change needs a PR versus an operational repo's config change can commit + straight to develop, bumping version.json, adding or dropping a release target, or reasoning + about why a merge did or didn't trigger a publish. Triggers even when the request sounds like + ordinary git housekeeping ("just push this config fix", "merge develop into main", "cut a + release"), because the two workflow models genuinely differ (a direct-to-develop commit that is + correct in an operational repo is a rule violation in a release repo, and vice versa) and + applying the wrong one is not obviously wrong to a reader who only knows one of the two. +--- + +# Operational vs. Release Workflow + +## Why this exists + +Two workflow models exist because the underlying repos are two different things. Most fleet repos +ship versioned units of delivery, so they earn a feature -> `develop` -> `main` flow with real +release gates. A handful of repos instead track a live service's running state (Home Assistant, +ESPHome, Vantage, home automation configs) where the "release" is the config already committed, +not something built and shipped later. Applying the release model's ceremony to an operational +repo, or skipping the release model's gates on a repo that actually ships versioned artifacts, is +each wrong in its own repo and correct in the other, which is why this is one skill keyed on which +repo you're in rather than two skills that never talk to each other. + +## Which model this repo uses + +Read the registry `workflowModel` field for this repo (`release`, the default, or `operational`). +The rest of this skill's "Branching" and "Publishing" sections describe the `release` model. The +"Operational repositories" section below is the complete delta for `operational` repos. Anything +not mentioned there is unchanged. When in doubt which one applies, check `registry/repos.json` +rather than guessing from the repo's contents. + +## Branching (release model) + +- **GitHub's repository setting for "default branch" reads `main`, but `develop` is where work starts and where in-flight content lives.** A worktree or clone that defaults to "the default branch" lands on `main` and can silently miss content that has merged to `develop` but not yet been promoted. Before branching off a change, or asserting something absent from this repo, check `develop`, not just whichever branch a tool defaulted to. See GOVERNANCE.md "Verification Discipline" on naming the branch a "does not exist" claim was checked against, and the `repo-worktree` skill, which owns the worktree-creation moment this base-branch choice is made at. +- `develop` is the integration branch. Feature branches -> `develop` is **squash-only**, which + keeps `develop` linear. +- `develop -> main` is **merge-commit only** (no squash, no rebase). Merge commits preserve + `develop`'s commit list as a real second-parent reference on `main`, which lets the release + model attribute releases to the develop commits that produced them. Branch protection enforces + this: the `develop` ruleset allows only `squash`, the `main` ruleset allows only `merge`. +- All commits on both branches must be cryptographically signed (SSH or GPG), see + `git-commit-conventions`. Squash and merge commits created via the GitHub UI are signed by + GitHub's web-flow key. +- **`develop` is forward-only, with no `main -> develop` back-merges.** The `develop` ruleset's + squash-only setting physically blocks merge commits on `develop`. Any historical back-merge + commits in `git log` predate this rule and must not be repeated. +- **Never delete `develop`, and take the EOL-only conflict by taking develop's side.** A + promotion PR's head *is* `develop`, so `--delete-branch` deletes it. An EOL-only conflict on a + workflow YAML file resolves on a throwaway branch off `main`, not on `develop`. Full recovery and + conflict-resolution commands: `references/branch-protection-and-promotion.md`. +- **A merge or release ends with worktree cleanup and the base clone on current `develop`.** Run the `repo-worktree` post-merge procedure after a feature squash merge. Run it again after a promotion or release completes, unless the user explicitly asks to retain a checkout or branch. Remove finished task, conflict-resolution, installer, and release helpers. Never delete `develop`, and never leave the base clone on `main` merely because `main` was promoted or released. +- **Issue-closing keywords (`Closes #N`, `Fixes #N`) go in the `develop -> main` promotion PR, not + the feature -> `develop` PR.** GitHub auto-closes an issue only when the closing keyword merges + into the **default branch** (`main`), so a feature -> `develop` PR merge never fires it. + Reference the issue in the `develop` PR body if useful, but the actual closing keyword belongs on + the promotion PR. Closing by hand is the ordinary route wherever the keyword cannot fire (a + promotion that already merged without it, or completed work with no promotion imminent), not a + repair for a botched promotion, cite the squash SHA and re-read that commit before closing. +- **Neither ruleset requires branches to be up to date before merging**, for different reasons on + each branch (a graph-based check that would fail every release on `main`, a check that stalls + bot auto-merge on `develop`). Detail: `references/branch-protection-and-promotion.md`. +- **Configuring branch protection: import the committed ruleset payloads, don't hand-build them.** + Exactly two rulesets, named `develop` and `main`. Full procedure, including the operational + `develop` payload and the brownfield-repo signing caveat: + `references/branch-protection-and-promotion.md`. +- **Dependabot and codegen target both `main` and `develop` in parallel**, each branch absorbing + its own bot PRs independently so neither falls behind, with the merge-bot dispatching the merge + form (`--squash`/`--merge`) that matches each PR's base ruleset. Codegen output must be + deterministic from its inputs alone, never per-run state, or the two branches' legs conflict on + every promotion. Full mechanics: `references/branch-protection-and-promotion.md`. +- **App-token workflows authenticate with Client ID, not the deprecated App ID.** Use + `client-id: ${{ secrets.CODEGEN_APP_CLIENT_ID }}` at any new App-token call site. + +## Publishing (release model) + +- **The two-phase model is the default: PRs build fast, publishing is batched.** A PR only + smoke-tests (unit tests plus a reduced build of the changed targets), it never pushes anything. + `publish-release.yml` is the sole publisher, and each run builds a **single trigger branch** + (`main` a release, `develop` a prerelease). +- **A human merge never auto-publishes.** Publishing fires on a **`workflow_dispatch`** of + `main`/`develop` (a human-initiated release), a **code-affecting bot push to `main`** (the + codegen App merging a Dependabot/codegen PR, gated on `github.actor` so a human + merge/promotion skips it), or a **weekly `schedule`** (Docker only, to refresh the base image). + A source-only repo publishes on dispatch only. +- **The changes-detection job is a required check that must succeed, not just not fail.** A + paths-filter error must never let a target-changing PR merge with its smoke build silently + skipped. A skipped smoke job (no matching change) passes, `failure`/`cancelled` blocks. +- **Versioning is semantic and maintainer-controlled.** `version.json`'s `major.minor` is the + version floor, edited by the maintainer for functional changes only, in the PR that introduces + the work, never on a fixed cadence or mechanically after a release. NBGV appends the git height + automatically on every commit, so a release always gets a fresh build version with **no + post-release bump** and no develop-ahead requirement. +- **Docs reference the 2-digit `major.minor` line, never a 3-digit build.** `README.md`, + `HISTORY.md`, and release notes name the version as `Version 1.0` (the floor), never the concrete + build height, which is both wrong (the real height differs) and a maintenance trap. + "Correcting" `1.0` to `1.0.0` is a defect. +- **A no-op publish (unchanged NBGV `SemVer2`) re-pushes nothing to any target keyed on the + version string, except Docker, which always re-pushes** to pick up upstream base-image + refreshes. Full guarantee and the `version.json` `pathFilters` boundary: + `references/release-publish-mechanics.md`. +- **Adding, dropping, or wiring a release target** (which leaf task, which artifact-naming + contract, which seam a given output belongs to: a GitHub Release asset, a package-registry push, + an image-registry push, a filesystem deploy, or a source-only repo with no build layer at all), + and tracking an upstream release from a wrapper repo: `references/release-publish-mechanics.md`. + See also `WORKFLOW.md` for the full CI/CD contract this section's rules are load-bearing + excerpts of. + +## Operational repositories (the complete delta) + +Everything above is the `release` model. An `operational` repo (registry `workflowModel: +operational`) tracks a live service's running state rather than shipping versioned units of +delivery, and differs from the `release` model in exactly these ways, everything not listed here +stays the same: + +- **Commit configuration directly to `develop`.** There is no feature branch requirement, the + maintainer commits straight to `develop`, and only *occasionally* opens a `develop -> main` PR to + bless a known-good snapshot. The `develop` ruleset drops the PR and status-check gate, so direct + signed pushes are allowed (force-push, deletion, and unsigned commits are still blocked), and CI + runs on the push as **advisory** feedback that never rejects a commit. +- **A PR into `develop` stays available, and CI runs on it, reported but not required.** Dropping + the requirement permits the direct push, it does not withdraw the pull request, so a change worth + reviewing takes one and both paths into `develop` are legitimate. +- **Take the pull request whenever the change is not one a reader takes in at a glance and + reverts cleanly.** What decides it is the shape of the change, not a line count: restructuring + rather than adjusting a value, touching several files at once, introducing a device, an + integration, or an automation that did not exist before, and anything whose failure shows up on + the live service rather than in a lint run are each the pull request case. So is a change the + author cannot state in one sentence. This stays a judgment call by design, adding a + `pull_request` rule to the operational `develop` ruleset would gate the direct push too and + withdraw the allowance the model exists to give. +- **The `main` promotion gate is unchanged.** The shared `main` ruleset still **enforces** the + required `Check pull request workflow status job` on the `develop -> main` PR. For an operational + repo that check is lint/validation only (editorconfig/EOL plus a domain linter such as a Home + Assistant or ESPHome config validation, never unit tests), so `develop` stays the live surface + and a broken config can never reach `main`. +- **Release only by manual dispatch.** Operational repos carry `releaseTrigger: dispatch-only` and + run no codegen or auto-publish bots, publishing **only** on a manual `workflow_dispatch` (the + same source-only release the publisher already supports: tag, source zip, README, LICENSE, + NBGV-versioned), never automatically. The `develop -> main` promotion just blesses a known-good + snapshot, a release is a separate, deliberate dispatch. +- **Fleet sync still applies.** Dependabot's dual-target sync and the App-signed merge-bot run on + **every** tier, operational included, so both branches stay in sync and a promotion stays a + clean forward merge. +- **Line-ending policy differs too**, following the consuming app's native platform rather than the + fleet LF default, per the registry `lineEndings` field. That rule belongs to + `comment-and-doc-style`, not repeated here. diff --git a/.github/skills/operational-vs-release-workflow/references/branch-protection-and-promotion.md b/.github/skills/operational-vs-release-workflow/references/branch-protection-and-promotion.md new file mode 100644 index 0000000..6d934c0 --- /dev/null +++ b/.github/skills/operational-vs-release-workflow/references/branch-protection-and-promotion.md @@ -0,0 +1,110 @@ +# Branch Protection and Promotion Mechanics + +Full detail for the "Branching" rules in `SKILL.md`. Load this when configuring or reconstructing +branch protection on a fleet repo, executing a `develop -> main` promotion, recovering a lost +`develop`, resolving an EOL-only promotion conflict, or working on the dual-target bot wiring +(Dependabot, codegen, the merge-bot), not for an ordinary feature-branch PR (the SKILL.md summary +covers that case). + +## Configuring branch protection: don't hand-build the rules + +Delete **all** classic branch-protection rules and stray rulesets because rulesets are the only +protection mechanism. From a hub checkout at `main`, create **exactly two rulesets named `develop` +and `main`** from the hub's `repo-config/*.json` payloads. Run +`repo-config/configure.sh apply <owner>/<repo> release|operational` from that checkout. The names +are load-bearing because governance content and workflows reference them. The registry +`workflowModel` selects the `develop` payload for a registered repository. Pass the model +explicitly for a repository outside the registry. See the hub's `repo-config/README.md` +"Rulesets" for the configured state. + +## Executing a `develop -> main` promotion safely + +Two traps, both learned the hard way: + +- **Never delete `develop`.** A promotion PR's head *is* `develop`, so `gh pr merge --delete-branch` + (and a repo's "Automatically delete head branches" toggle, kept off in the hub's + `repo-config/settings.json` for exactly this reason) deletes `develop` itself. Merge a promotion + with a plain `gh pr merge --merge`, no `--delete-branch`. If `develop` is ever lost this way, + restore it to the merged PR's head SHA, which is still reachable as the merge commit's second parent: + `gh api -X POST "repos/<owner>/<repo>/git/refs" -f ref=refs/heads/develop -f sha="$(gh pr view <n> --json headRefOid --jq .headRefOid)"`. +- **Spurious EOL-only conflicts resolve by taking `develop`.** When `develop`'s `.editorconfig` + line-ending default has changed (for example the fleet-wide CRLF-to-LF flip) while `main` hasn't + caught up yet, `develop -> main` conflicts *whole-file* on every renormalized path. + `develop`'s `required_linear_history` plus PR rulesets forbid resolving on `develop` (no merge + commit, no force-push), so resolve on a throwaway branch off `main`: + `git checkout -b promote/develop-to-main origin/main && git merge origin/develop`, take + `develop`'s side for the EOL-conflicted files (`git checkout --theirs <file>`) **after + confirming each is content-identical modulo EOL, or that `develop` is a strict superset** + (`diff <(git show ":2:<file>" | tr -d '\r') <(git show ":3:<file>" | tr -d '\r')`), then open that branch into + `main`. Verify no genuine `main`-only content is dropped (build/test where the repo supports it). + +## Why both rulesets omit "Require branches to be up to date before merging" + +The flag is off on `main` and on `develop`, for related but distinct reasons. + +- **Main**: the check is graph-based, it asks whether `main`'s tip commit is reachable from + `develop`, not whether the two branches have the same content. After any `develop -> main` + release, `main`'s tip is a brand-new merge commit that `develop`'s history doesn't contain. + Forward-only `develop` never adds it (no back-merge of `main` into `develop`), so the check + would fail on every subsequent release. Other technical workarounds (rebasing `develop` onto + `main`, or rewriting `develop`'s history) exist but contradict the squash-only `develop` ruleset + and the linearity invariant. +- **Develop**: the check stalls bot auto-merge when two bot PRs against `develop` land within the + same window. As soon as the first merges, the second flips to `mergeStateStatus: BEHIND` and + GitHub's auto-merge will not fire while strict is on. The merge-bot only *enables* auto-merge on + `opened`/`reopened` and never auto-updates bot branches, and Dependabot's rebase isn't real-time, + so the second PR sits OPEN with all checks green indefinitely. Squash mechanics still rebase the + diff onto `develop`'s tip on merge, `required_linear_history` still enforces linearity, textual + conflicts still block `mergeable: CONFLICTING`, and the required `Check pull request workflow + status job` still gates merges. The only thing lost is pre-merge detection of + *semantic-but-not-textual* conflicts, which the post-merge `develop` CI run catches anyway. + +## Dual-target bots + +**Dependabot and codegen target both `main` and `develop` in parallel.** +`.github/dependabot.yml` duplicates every ecosystem entry (one per branch) and the codegen +workflow runs as a matrix over both branches with branch names `codegen-main` and +`codegen-develop`. Each branch absorbs its own bot PRs independently, so neither falls behind, and +the forward-only rule still holds, nothing is back-merged from `main` to `develop`, both branches +receive their updates directly. The merge-bot (`.github/workflows/merge-bot-pull-request.yml`) +dispatches `--squash` or `--merge` from each PR's base ref via a `case` statement so the form +matches the ruleset on either base. Dependabot **security** PRs (CVE-driven) always open against +the repo default branch (`main`) regardless of `target-branch`, and the same `case` statement +covers them. The merge-bot auto-merges **every** Dependabot tier including semver-major (no +ecosystem or update-type guard), the required CI checks are the gate, not the bump magnitude, so a +major that breaks the build fails its checks and never merges. + +**Why parallel dual-target rather than develop-only with eventual flow-through:** +push-distribution channels (HACS for Home Assistant integrations, Linux distros that vendor from +`main`, etc.) consume `main` directly. A develop-only model would leave `main` running stale code +during long-running develop features. Codegen content can also be production-critical (live +API-derived data, language lists, build catalogs) rather than just sample/demo content, so both +branches need fresh codegen on their own cadence. + +**Maintainer-pushed commits on a bot PR auto-disable auto-merge.** The merge-bot's +`merge-dependabot` and `merge-codegen` jobs only fire on `opened`/`reopened` events (auto-merge is +enabled exactly once per PR). When a maintainer pushes commits to a bot's branch (a `synchronize` +event with an actor that isn't the same bot), the merge-bot's +`disable-auto-merge-on-maintainer-push` job fires and calls `gh pr merge --disable-auto`. The +maintainer's commits stay in the PR but won't auto-merge with the bot's content. Re-enable +auto-merge manually (`gh pr merge --auto <PR>` or the GitHub UI) when ready. + +## Codegen determinism + +The codegen workflow is a mechanism to refresh files that are checked into the repo: it runs a +matrix over `main` and `develop`, each leg regenerating against its own checkout and opening its +own PR (`codegen-main -> main`, `codegen-develop -> develop`). For the two legs not to conflict on +`develop -> main`, the generated output must depend only on its inputs, never on per-invocation +state (timestamps, GUIDs, build IDs), which would diverge every run and conflict on every release. +**What** a repo regenerates (data files, source, or both) and **how** (download and process an +external source, transform local inputs, whatever) is entirely its own concern. The constraint is +only that the output be input-deterministic, not how it is produced. A repo adopting codegen +supplies its own input-deterministic generator and wires the codegen reference workflow +(`run-codegen-pull-request-task.yml` and its scheduler). + +## App-token workflows use Client ID, not App ID + +`actions/create-github-app-token` deprecated the numeric `app-id` input in v3.0.0. Use +`client-id: ${{ secrets.CODEGEN_APP_CLIENT_ID }}`. When adding new App-token call sites, use the +same form, and do not reintroduce `app-id` / `CODEGEN_APP_ID`. See the hub's +`repo-config/README.md` "Secrets" for which secrets each mechanism needs. diff --git a/.github/skills/operational-vs-release-workflow/references/release-publish-mechanics.md b/.github/skills/operational-vs-release-workflow/references/release-publish-mechanics.md new file mode 100644 index 0000000..ad7253f --- /dev/null +++ b/.github/skills/operational-vs-release-workflow/references/release-publish-mechanics.md @@ -0,0 +1,135 @@ +# Release Build and Publish Mechanics + +Full detail for the "Publishing" rules in `SKILL.md`. Load this when adding or removing a release +target, wiring a new leaf build task, deciding where a build output belongs (a GitHub Release +asset, a package-registry push, an image push, a deploy), or setting up a wrapper repo that tracks +an upstream release, not for reading the release model's shape (the SKILL.md summary covers that). + +## Reusable-task parameter contract + +Every `build-*-task.yml` and `build-release-task.yml` takes `ref` (git ref to check out/version), +`branch` (logical branch driving config/tags/prerelease, where `main` => Release/`latest`/ +non-prerelease, else Debug/`develop`/prerelease), and where relevant `smoke`. +**Branch-derived config keys off `inputs.branch`**: each run builds one branch, and the top-level +publisher passes `branch: ${{ github.ref_name }}`, which the tasks forward and read as +`inputs.branch` (not `github.ref_name`) for config/tags/prerelease. `get-version-task.yml` takes a +`ref` so NBGV versions the right branch. + +## Per-target subsetting + +`build-release-task.yml` is a hub-hosted task with per-target `enable_*` inputs, so a repo drops a +target by setting its `enable_<target>: false` at the caller stub rather than deleting a job: the +hub task carries the full job graph for every repo, and the caller stub's `with:` block is where +the target list is expressed. A repo still curates its path-filter entry in +`test-pull-request.yml`, and (for PyPI) the `publish-pypi` job in its own `publish-release.yml`, +since `id-token: write` belongs at that one entry point. CodeGen, versioning, badge, merge-bot, +and Dependabot are target-agnostic. + +## Orchestration vs. build: the override seam + +The pipeline splits into two layers. The **orchestration** layer is generic and is the +standardization baseline: `publish-release.yml` (single-branch publish plan), the `get-version` +task plus `github-release` job inside `build-release-task.yml`, `get-version-task.yml`, and the +aggregator shape of `test-pull-request.yml`. Within +`test-pull-request.yml`, only the `changes -> smoke-build -> check-workflow-status` aggregator +wiring and the ruleset-bound job name are verbatim orchestration, while the `unit-test` job and +the `dorny/paths-filter` entries are owned/per-target. The **build** layer is a hook: a composite +action at `.github/actions/build-<target>` the hub-hosted `build-release-task.yml` reaches. The +hub defaults require explicit project paths. A project needing more than a path override carries +its own hook. + +The contract that keeps the seam clean: **a target contributes files to the GitHub release by +uploading a workflow artifact named `release-asset-<branch>-<target>`.** The `github-release` job +collects every `release-asset-<branch>-*` artifact by pattern, so its `download-artifact` step +uses `pattern:`/`merge-multiple:`, **never an `artifact-ids:` that names a build job's output** +(the producing build jobs still appear in `needs` for sequencing). That makes the tag-the-commit +plus create-the-release plus attach-the-assets logic reusable **as-is** across repos. **This +name-pattern handoff is canonical for every repo, single-target included**: name your one asset +`release-asset-<branch>-<target>` and the verbatim `github-release` globs it. Do not switch a +single-target repo to an `artifact-id` output plus `download-artifact` `artifact-ids:`, which +looks tidier for 1:1 but forks the `github-release` download and breaks its verbatim carry. + +**What a repo still curates** (by design, not a leak): which `enable_<target>` inputs its caller +stub sets, per the per-target subsetting rule above. `build-release-task.yml` is hub-hosted +(`docs/reusable-workflows.md` "Stage 4: The Release Chain and the Docker Core"), so its job graph +and its `github-release` job are the hub's, not a per-repo file a caller edits. A repo adopting the +release chain carries only the caller stub in its own `publish-release.yml` and +`test-pull-request.yml`, naming the hub task by pin and setting the `enable_*`, `docker_image`, +and project-path inputs its targets need. + +## Map your outputs to the right seam + +Pick by where each artifact *goes*, not by language: + +- **Files attached to the GitHub Release** (zips, binaries, packaged libraries): a dotnet-publish + hook or a build-nuget hook per output, each uploading `release-asset-<branch>-<name>`. This is where the + .NET `dotnet publish` or `dotnet build` and package push lives. The hub default takes an explicit + project path, and a project needing different build behavior replaces the hook. A data-only + repo's own output (e.g. a symbol library) is not yet + expressible as a hub hook or an `enable_*` input, so it stays a carried leaf until the hub task + grows one. +- **Package-registry pushes** (NuGet.org, PyPI): the target both builds **and** publishes to its + registry. NuGet pushes from inside the build-nuget hook (OIDC trusted publishing through + `NuGet/login`, no stored API key) *and* also uploads a `release-asset-*` (.7z) for the GitHub + release. PyPI is split: the build-pypi hook only builds and uploads the + `pypi-build-<branch>` artifact, and the separate `publish-pypi` job in the caller's own + `publish-release.yml` does the OIDC Trusted-Publishing upload (`id-token: write` is granted only + at that one entry point), and PyPI contributes **no** `release-asset-*`. +- **Image-registry pushes** (Docker Hub): `build-docker-task.yml`, hub-hosted like + `build-release-task.yml`, pushes multi-arch tags directly and contributes **no** + `release-asset-*`. The image set comes from a docker-prepare hook (the hub default emits the + single vanilla entry an `image` input implies). A multi-image or upstream-pinned repo carries its + own hook, and a shared base layer comes from a required docker-build-base hook with no hub + default. To publish the Docker Hub repository overview, the hub-hosted `publish-docker-readme-task.yml` + pushes a readme via `peter-evans/dockerhub-description` (single-repo by default, matrix per + image for multi-image repos), wired into `publish-release.yml` and gated to `main` both by the + caller's `branch` input and inside the task itself. A `docker-readme-transform` hook sets a + `readme-filepath` step output naming which file to push, defaulting to `Docker/README.md` if + present else `README.md` as-is, so a repo needs a hook only to render the file first or to + override that default. +- **Filesystem on a host the project owns** (a static site, a config tree): a deploy leaf builds + the tree and ships it over the repo's own transport, contributing **no** `release-asset-*`. It + is a **separate `workflow_dispatch`** from the release, so a redeploy of an unchanged commit + mints no tag, and its credentials come from a **per-environment GitHub Environment** rather than + the repository secret store. Its last step asserts what the host actually serves, the release id + and the environment, never that the transport exited zero. Retention at the destination is + bounded by a declared count, and one side is recorded as owning the prune: the deploy where its + credential can observe the destination, the host where that credential is deliberately + write-only. +- **Source-only / no build** (validate + tag + release): the repo has no leaf build tasks. + Its dispatch-only `publish-release.yml` calls the hub-hosted `build-release-task.yml` after the repo's reusable validation task succeeds. + The caller sets `github: true`, every `enable_*` input to false, and `expect_release_assets: false`. + The reusable task runs NBGV and creates the release with the tag, automatic source archive, README, and LICENSE. + +`get-version-task.yml` installs the .NET SDK only because NBGV needs the runtime to compute the +version/tag, which is heavyweight but expected even for a non-.NET repo, and acceptable as-is. + +## No-op republish guarantee + +A weekly/dispatch publish where NBGV `SemVer2` is **unchanged** (no new commit since the last +publish) re-pushes **nothing** to GitHub Releases (the `github-release` job's `release-exists` +check skips the create step), NuGet (`dotnet nuget push --skip-duplicate`), or PyPI +(`gh-action-pypi-publish` `skip-existing: true`), since all three key on the version string. +**Docker always re-pushes** by design: it picks up upstream base-image refreshes (e.g. +`ubuntu:rolling`) that aren't visible in the repo. Boundary: `version.json` has **no +`pathFilters`**, so *any* commit, including a CI/workflow-only or docs-only change, advances the +NBGV git height and therefore `SemVer2`, and the next publish *does* create a fresh release for it +even when the shipped binary is byte-identical. This is accepted NBGV behavior, and `pathFilters` +are intentionally not added. + +## Wrapper repos that track an upstream release + +A repo wrapping an upstream release uses the hub-hosted `check-upstream-version-task.yml`: a +required `resolve-upstream` hook sets a `versions` step output, a **JSON object of +`name -> version`**, written to a committed state file at the **repo root beside `version.json`** +(default `upstream-version.json`, since it is a build-input version source, not GitHub-platform +config, so it does not belong under `.github/`), and opens a rolling App-signed bump PR per branch +that the merge-bot auto-merges (`merge-upstream-version`). The object carries one key for the +common single-version case (`{"version": "X"}`) or N keys for a wrapper that pins several upstream +components (e.g. an image plus a companion tool), and the build reads each component by key, and +the bump PR's title/body name only the keys that actually moved. Call it from a scheduled +entry-point workflow and matrix only the branches that ship the version (a CI-only version uses +`["develop"]`). A merged bump ships on the **next publish**, not immediately, which is the +two-phase latency tradeoff. A tracker whose bump needs a human decision instead of auto-merge, for +example one that snapshots a package list to review rather than a version to adopt outright, sets +`auto-merge: false`, which prefixes the head so no merge-bot rule matches it. diff --git a/.github/skills/pr-review-conduct/SKILL.md b/.github/skills/pr-review-conduct/SKILL.md new file mode 100644 index 0000000..810955e --- /dev/null +++ b/.github/skills/pr-review-conduct/SKILL.md @@ -0,0 +1,178 @@ +--- +name: pr-review-conduct +description: >- + Governs opening, driving, and merging a pull request review loop in a ptr727/ProjectTemplate + fleet repo: requesting a review after a push, triaging findings (including suppressed + low-confidence ones), replying and resolving threads, and deciding whether a PR is actually + mergeable. Use this whenever about to open a PR, immediately after creating one, about to merge + a PR, enable auto-merge, ask the maintainer for merge permission, push a fix and move on without + re-checking review state, or judge a PR "green" or "clean" from CI or mergeStateStatus alone. + Triggers even when the request sounds routine, such as "open a PR," "merge this," or "it's all + green, go ahead," because PR creation starts the review loop and mergeStateStatus: CLEAN + can go clean once checks pass and every known thread is resolved, while still saying nothing + about whether the review that resolved those threads covered the current head SHA, read the + full diff, or left a suppressed low-confidence finding, which opens no thread at all, + unanswered. Also triggers when a review loop looks stuck + (no review landing, findings that keep reappearing) or when deciding a finding is real, false, + deferred, or a deliberate decline. Provider-specific mechanics are implemented by + scripts/pr_review.py and bootstrapped by .github/copilot-instructions.md. This skill is the + contract those surfaces implement, not a replacement for them. +--- + +# PR Review Conduct + +## Why this exists + +`mergeStateStatus: CLEAN` reflects required status checks and any review thread the ruleset's +conversation-resolution requirement already tracks as resolved. It says nothing about whether the +review that resolved those threads actually covered the **current** head SHA, whether it read the +full diff rather than part of it, or whether a suppressed low-confidence finding, which never +opens a thread for the ruleset to see, was ever answered. A PR that looks done, green checks, no +visible comments, routinely still carries a finding nobody has answered. Treating "green" as +"mergeable" is the single most common way this loop gets skipped. + +## Merge Gate, check this before merging or enabling auto-merge + +**Do not merge, and do not enable auto-merge, unless ALL of these hold:** + +1. Required status checks are green, and where they are not, the reason is **read**, never + inferred. `BLOCKED` covers a failed check, a required check nothing is running, an unresolved + thread, and a missing approval alike, and the response differs by cause. +2. A review is confirmed on the **current head SHA**, matched by commit SHA rather than assumed + from a green merge-state. A push makes checks go green *before* the re-review lands, and the + matched review is **read**, not just counted. A review can carry the head SHA and still decline + the PR outright, or say it read only part of the changed files. `pr_review.py`'s + `review_on_head` names Copilot's own coverage specifically, the currently required reviewer, + not "no review of any kind covers this head": a trialed advisory reviewer (CodeRabbit, + Qodo) carrying the exact head under `other_reviewed`, with an empty review body and no new + threads, is its own ordinary "reviewed, nothing to flag" shape, not a missing review (#1066). +3. **Every** finding on that head SHA is closed: threads resolved, issue-level comments (which + have no resolve action) triaged and replied to, **and** the low-confidence findings collapsed + in the review body investigated and answered. Those appear in no thread, so polling threads + alone reports a clean pass while they stand. The same holds for CodeRabbit's own + "outside diff range" comments (`cr_outside_diff` in `pr_review.py`'s digest) and for Qodo's + comment-only findings (`qodo_open`): neither opens a `reviewThreads` entry either, so + give each one the same triage the low-confidence findings above already get (#1058). Qodo's own + `Resolved`/`Dismissed` self-tracked badge is a fast pre-triage signal, not a substitute for + reading the finding, spot-verify against `gh pr diff` rather than trusting it outright. +4. Nothing in the review was a shape the tooling could not read (an unrecognized heading, a moved + section, an unfamiliar coverage wording). An unrecognized shape blocks the gate on its own. + File an issue naming it and quoting the body, rather than guessing what the new wording + probably meant. +5. The maintainer has given **explicit** permission to merge. + +The agent never merges on its own. A green or CLEAN PR with one open finding is not mergeable, +full stop, whatever the merge-state field says. + +## Expected review loop + +Open every fleet-owned pull request ready for review. Draft state delays the loop and causes +reviewers to skip, so it has no place in the internal feature-to-develop or develop-to-main +workflow. The separately documented `upstream-contribution-workflow` may use a draft while a +third-party contribution is still being prepared for upstream review. + +Opening a pull request starts this loop by default. Creating the PR is not a terminal handoff. +Only an explicit maintainer instruction may stop, defer, or alter the loop. Silence or a request +that says only "open a PR" is not such an instruction. + +Run every `scripts/pr_review.py` command below from a hub checkout. The script is hosted there and +is never carried into a downstream repository. + +Run `local-strict-review` against the branch's current diff before step 1's push, and again before any fix push under outcome 1 below. + +1. Push changes to the PR branch and open the pull request when it does not exist. +2. Run `scripts/pr_review.py status` once in the foreground and read its output. +3. Re-request a review for the **current head SHA**. Auto-trigger is unreliable, so request it + explicitly (mechanics in the Copilot runbook). The UI is a fallback only. +4. Run a bounded `scripts/pr_review.py wait` in a background process and read its terminal output. + A completed review raising **no findings** is a valid terminal outcome, so do not re-trigger it + or read silence as a missing review. A review whose body says it declined to review is the one + exception, and it is terminal the other way. Nothing follows it, and re-requesting the same + head only repeats the decline. +5. Triage findings (see below). +6. Apply fixes or write a rationale for declines. +7. Reply to each thread and resolve what was addressed. +8. Re-run the loop after every fix push until the checks are green and no finding remains open. + +The review effort setting is user-controlled. The workflow never selects or changes it. `status` reports `Lite`, `Balanced`, or `Max` when the completed review exposes that metadata, and distinguishes an inherited `Default (<level>)` from an explicit choice. Missing effort metadata reports `unknown` and does not change coverage or completion. A pending effort-labeled request can complete without a `copilot_work_started` timeline event, so absence of that event never proves the request is abandoned. The bounded timeout reports `PENDING` when no review or terminal answer arrives. After a timeout with `requested=yes`, rerun `wait` for another bounded interval by default because the request may still be active. If the maintainer directs a retry, remove Copilot in the pull request UI, add it again, and rerun `wait`. This recovery replaces only the review request and never changes the effort setting. + +Drive to green, a review confirmed on the latest head SHA and every actionable finding closed, +then apply the Merge Gate above. **Never exit the loop early.** A round count is not a stopping +condition, and neither is patience running out. Reporting only that the PR was opened is an early +exit unless the maintainer explicitly instructed the agent not to monitor or drive its review. + +After an authorized merge, run the `repo-worktree` post-merge cleanup procedure unless the user explicitly asks to retain the checkout or branch. The pull request loop is incomplete while its finished worktree or local task branch remains. It is also incomplete until the base clone returns to fetched and fast-forwarded `develop`. + +## Every finding ends in one of five outcomes + +1. **Real, so fix it.** Run `local-strict-review` against the branch's current diff before pushing + the fix, then reply with the fixing commit SHA. For a finding on platform-specific code + (PowerShell, a macOS- or WSL-only path), "fixed" means executed on that platform, per + `agent-conduct` "Before Claiming Done": a fix reasoned out by analogy to a tested equivalent + elsewhere is not yet fixed, and the reply says so rather than claiming the SHA closes it. +2. **Not real, or real but structurally out of scope, so decline in the thread with evidence.** + Disprove a wrong finding with the command and its output, the code path that makes it + impossible, or the rule that governs it. A finding that is factually correct but not this + repo's to fix (a verbatim-fidelity manifest entry byte-locking the section, ownership that + sits elsewhere) declines the same way: name the boundary and cite what proves it. Either shape + closes the thread on its own evidence. An assertion ("this is fine") does not close a finding, + a decline needs evidence the reviewer itself could check. +3. **Real, fixable here, but deliberately left as is, a value call rather than a scope + boundary, so it is the maintainer's, not the agent's.** Reach for this only once outcome 2 is + ruled out, since a scope boundary declines on its own evidence and never needs this outcome at + all. State the finding and why the fix is unwanted, and get an explicit answer in the same + turn, before moving to other work. A plan to ask later is resolution by silence the moment + attention moves elsewhere. If the maintainer is not reachable right now, leave the thread open + and say so, rather than treating the intention to ask as the asking. +4. **Real and worth doing later, so file the issue first, then reply with its link.** A deferral + noted only in a thread is lost the moment the PR merges. +5. **Keeps recurring, so fix the class, not the instance.** A finding raised repeatedly against + correct code means the code is not communicating something: add the comment, sharpen the name, + narrow the interface, or fix the rule if the rule is wrong. Bouncing the same point across + rounds is the signal to escalate the rule itself, not to keep re-arguing it. + +**A disposition decided on one PR does not carry to the next.** The same finding shape recurring +on a sibling repo or PR, even within one batch or one session, gets its own outcome: its own +evidence-backed decline (outcome 2) or its own explicit maintainer answer (outcome 3). A prior +instance's outcome is context for the new one, never a standing answer to reuse in its place. + +## Triaging findings + +**A low-confidence (suppressed) finding is not a low-value one.** Judge each against the code, +never against its confidence label. Classify before responding: + +- **Bug**, wrong behavior, missing coverage, a real code or doc divergence. Fix it. +- **Style or convention**. If the cited rule matches the existing tree, fix the code. If the rule + contradicts the tree or industry norm, **fix the rule, not the code**, and take it to the + maintainer (outcome 5) rather than bouncing the same code across rounds. +- **Architectural opinion**, a proposed redesign. Surface it with a recommendation, never apply + it unilaterally. + +## Answering a suppressed finding + +A suppressed finding has no thread and no resolved or unresolved state, so an answer needs to +carry its own context: quote the finding (with its `file:line` anchor and enough of the +reviewer's own words to identify it), give one bold verdict per finding (`Fixed in <SHA>`, +`Disproven`, or `No change needed`), state the `(N)` count the block gave so answers can be +checked against findings, and link the review round. **Read every round, not only the head.** A +suppressed finding does not retire when a later push supersedes it, it just stops showing up in a +head-scoped query while still unanswered. Post the answer with `scripts/pr_review.py comment` +from a hub checkout. Do not use a provider connector or reconstruct the GitHub mutation. + +## Escalate to the maintainer when + +- A genuine design trade-off surfaces (fail-open vs. fail-closed, refactor scope). +- A finding keeps recurring. Bring the pattern and a recommended fix (rule change or code + change), don't keep silently re-declining it. +- A finding is judged real but should not be fixed. That decision is never the agent's alone. +- An architectural redesign is proposed rather than a bug fix. + +## Mechanics Live Elsewhere + +This skill is the provider-agnostic contract. Use `scripts/pr_review.py` from a hub checkout for +the GitHub-specific API operations. `status` reports coverage, threads, body-only findings, and +shapes in one call. `wait` requests and polls in-process. `comment` posts a PR-conversation +answer after it reads the PR node ID. `reply` resolves a thread by matching the finding's own +words instead of a line number a fix push can move. The repository's +`.github/copilot-instructions.md` bootstraps Copilot into the `code-review` skill and its stable +coverage marker. Do not reconstruct the API operations by hand. diff --git a/.github/skills/python-codestyle/SKILL.md b/.github/skills/python-codestyle/SKILL.md new file mode 100644 index 0000000..f7a7451 --- /dev/null +++ b/.github/skills/python-codestyle/SKILL.md @@ -0,0 +1,185 @@ +--- +name: python-codestyle +description: >- + Governs Python code style for ptr727/ProjectTemplate fleet repos: the build-versus-lint-only + profile split, the uv/ruff/pyright/mypy/pytest toolchain, src layout, formatting and linting, + comment and docstring conventions, type hints, naming, imports, patterns to avoid, test + conventions, and versioning. Use this whenever writing, reviewing, or editing a .py file, a + pyproject.toml, or a uv.lock, whenever running or choosing a Python formatting, lint, type-check, + or test command, whenever deciding whether a Python subtree is a shippable project or a lint-only + scripts tree, whenever choosing pyright versus mypy for a repo's CI gate, or whenever writing or + reviewing a Python test. Triggers even when the task looks like a small local fix ("just add a + helper function", "silence this lint warning", "add a dependency") or verification step ("run + the tests"), because choosing pytest before reading the profile turns an intentional unittest + suite into a false missing-dependency diagnosis. Applies only to a repo's Python side, a repo + with no Python has no use for this Skill. +--- + +# Python Codestyle + +## Why this exists + +This is the Python-specific half of the fleet's code style guide, kept in one place instead of +re-derived per repo or per session. CODESTYLE.md's General section still owns the rules every +language shares (clean-compile verification as a concept, the suppression-scope order, tooling +casing in prose), this Skill is everything specific to a Python project on top of that: the two +profiles, the toolchain, layout, and the language-level conventions. + +## Two profiles + +Read the repo's `OPERATIONS.md` local-verification commands before substituting a generic command. +Then read the `pyproject.toml` shape and pick the profile before running Python tooling or tests: + +- **build** (Project): `[project]` + `[build-system]` + committed `uv.lock`. Uses `uv run`, pytest, + pyright strict (or mypy where the repo requires it). +- **lint-only** (Scripts): no `[project]`, no lockfile. Uses `uvx` for third-party tools, unittest + for tests, and mypy as the CI gate. Do not run pytest or diagnose its absence as an environment + defect. Use the repository's exact coverage command and unittest scope from `OPERATIONS.md`. + +For the full profile specification and per-repo adaptation axes (type checker, dependency +declaration, versioning, VS Code config), see `references/profiles.md`. + +## Toolchain + +| Tool | Role | Config | +|---|---|---| +| [uv][uv-link] | env, deps, build, publish (build/publish only where the repo ships a package) | `pyproject.toml` `[dependency-groups]` or `[project.optional-dependencies]`, `uv.lock` | +| [hatchling][latest-link] | build backend (published packages) | `pyproject.toml` `[build-system]` | +| [ruff][ruff-link] | lint + format + import sort | `pyproject.toml` `[tool.ruff]` | +| [pyright][pyright-link] | type checker (the default, a strict baseline) | `pyproject.toml` `[tool.pyright]` | +| [mypy][mypy-link] | additional/alternate type checker (optional, the CI checker in a mypy-in-CI repo, required for Home Assistant) | `pyproject.toml` `[tool.mypy]` (or per home-assistant/core) | +| [pytest][docs-link] | test runner (build profile only, lint-only uses `unittest`) | `pyproject.toml` `[tool.pytest.ini_options]` | + +**Type checking targets strongly typed, deterministic code.** pyright in strict mode is the +default baseline on first-party code (a repo may instead run mypy in CI and keep pyright +editor-only via Pylance, per the next paragraph): `[tool.pyright]` `strict = ["src"]`, or the +integration package for a Home Assistant repo, with tests run in standard mode. pyright is the +anchor because Pylance embeds it, so the editor and the CLI/CI (`uv run pyright`) run the same +engine and never disagree. The standalone `ms-pyright.pyright` extension stays in +`unwantedRecommendations` because Pylance covers it. Relax strictness on third-party code only +when a dependency has no usable types and no alternative (e.g. `pandas`): a targeted, commented +`# pyright: ignore[...]` or a scoped `[tool.pyright]` override, never a blanket relaxation. + +**mypy is allowed, and required where the ecosystem demands it, it is not banned.** Running more +than one checker is normal when each serves a purpose (the .NET side pairs CSharpier and +`dotnet format` the same way), and pyright's inference and mypy's plugin ecosystem (e.g. +`pydantic.mypy`) catch different classes of error. A Home Assistant integration runs +`mypy --strict` because the platinum `strict-typing` quality-scale tier requires it, and a +pydantic-heavy library may opt in for the plugin. When a repo uses mypy it runs in CI and the +editor (the `ms-python.mypy-type-checker` extension) so the two stay consistent, and its mypy +command joins the clean-compile. A repo with no such need stays pyright-only, which is lighter and +inherently consistent. + +## Local development loop + +From inside a **build**-profile Python project directory. A **lint-only** Scripts profile has no +`uv.lock` to sync and no pytest to run, substitute `uvx` per tool and `unittest` per the Two +Profiles section above: + +```sh +uv sync # creates .venv, installs deps + dev group +uv run ruff format # auto-format +uv run ruff check --fix # auto-fix lint +uv run ruff check # verify lint clean +uv run ruff format --check # verify format clean +uv run pyright # verify types +uv run pytest # run tests +uv build # produce wheel + sdist in ./dist (published packages only) +``` + +The **build**-profile Python clean-compile is `uv run ruff format` + `uv run ruff check` + the +repo's type checker: `uv run pyright`, or `uv run mypy src` where mypy is the CI checker, or both +where the repo runs both (see Type checking above). Run it, plus `uv run pytest`, before +committing. A **lint-only** profile's clean-compile substitutes its `uvx` and `unittest` +equivalents, per Two Profiles above, and has no such command to run before committing beyond +those. These are documented commands, and an optional VS Code tasks mirror (all `type: process`, +no `&&` shell chaining, so it runs the same on any task shell) is in the hub +`vscode-tasks-python.json` snippet. CI runs the same clean-compile commands as the authoritative +backstop. A working local hook is strongly suggested, not opt-in: wire the Python `pre-commit` +framework from the canonical `catalog/snippets/pre-commit/.pre-commit-config.yaml`. See +GOVERNANCE.md "Running the Linters Locally" for what the hook must cover and what its absence +means. + +A restricted executor gives each task a cache directory under a writable temporary root. Point +`UV_CACHE_DIR`, `RUFF_CACHE_DIR`, `MYPY_CACHE_DIR`, and `COVERAGE_FILE` into that directory before +running the applicable tools. This keeps their generated state outside both the home directory +and the checkout. Do not change `HOME` or an agent configuration directory. A denied network +request means the tool did not run, so preserve the denial and rerun through the executor's scoped +approval mechanism. + +## Layout + +`src` layout, which keeps the package out of the repo root and prevents accidental imports of +unbuilt code: + +```text +<python-project>/ + pyproject.toml + README.md + uv.lock # committed for reproducible CI + src/ + <package_name>/ + __init__.py + _version.py # published packages; a source-only repo uses a static version instead + <modules>.py + tests/ + __init__.py + test_<module>.py +``` + +## Code style + +Key rules for every Python task: + +- **`ruff format` is authoritative.** Don't argue with the formatter. Configure in `pyproject.toml` + `[tool.ruff]`, not via inline `# fmt:` directives. +- **Run `ruff check --fix` before committing.** The configured rule families are in + `[tool.ruff.lint]` `select`. Add new rule families project-wide, not scattered inline `# noqa`. +- **`# noqa` is a last resort.** Scope it narrowly (`# noqa: E501`) with a comment. Recurring + false positives belong in `[tool.ruff.lint]` `ignore` or `per-file-ignores`. +- **All public APIs are typed.** Use modern syntax (`list[int]`, `X | None`). Don't add + `# type: ignore` without an explaining comment. +- **Don't add backward-compat shims.** Just delete unused code. Git history is the audit trail. +- **Don't add error handling for impossible cases.** Trust internal code. Validate only at boundaries. + +For comments, docstrings, full type-hint rules, naming, imports, and all patterns to avoid, see +`references/code-style.md`. + +## Tests + +`uv run pytest` for a build profile, `unittest` for a lint-only Scripts profile (see Two Profiles +above). One test file per module (`test_<module>.py`). A build profile prefers fixtures over +`unittest`'s `setUp`/`tearDown` lifecycle hooks. A lint-only profile uses those hooks directly, +since `unittest` has no fixture-injection mechanism of its own. Fakes over mocks either way. Test +the docstring's contract, not implementation details. See `references/testing.md` for the full +build-profile conventions, and `references/profiles.md` for the lint-only `unittest` conventions. + +## Versioning + +Published packages use `_version.py` with `__version__ = "0.0.0"` as a placeholder. Wire +`hatch-vcs` or equivalent to increment, publish with `skip-existing: true`. Source-only repos use +a static `version` in `[project]` with no `_version.py`. See `references/profiles.md` for details. + +## Linter cleanliness + +Before pushing or opening a PR: + +- VS Code's Problems pane should be quiet for the files you touched. The relevant linters are ruff + (via the `charliermarsh.ruff` extension) and pyright (via the `ms-python.python` extension's + bundled Pylance). +- The **build**-profile CI gate is `uv run ruff check`, `uv run ruff format --check`, the repo's + type checker (`uv run pyright` or `uv run mypy src`), and `uv run pytest`, the same commands as + the local loop above, run from the Python project directory (invoked as separate steps, not + `&&`-chained, so the runner shell is irrelevant). A **lint-only** profile's CI gate is its `uvx` + equivalents plus its `unittest` suite, per `references/profiles.md`. +- Markdown in this directory follows CODESTYLE.md's repo-wide Markdown and Spelling rules, + packaged as the `comment-and-doc-style` Skill. + +<!-- External --> + +[docs-link]: https://docs.pytest.org/ +[latest-link]: https://hatch.pypa.io/latest/ +[mypy-link]: https://mypy-lang.org/ +[pyright-link]: https://microsoft.github.io/pyright/ +[ruff-link]: https://docs.astral.sh/ruff/ +[uv-link]: https://docs.astral.sh/uv/ diff --git a/.github/skills/python-codestyle/references/code-style.md b/.github/skills/python-codestyle/references/code-style.md new file mode 100644 index 0000000..9ad17f2 --- /dev/null +++ b/.github/skills/python-codestyle/references/code-style.md @@ -0,0 +1,93 @@ +# Python Code Style: Full Reference + +## Formatting and linting + +- **`ruff format` is authoritative.** Don't argue with the formatter, and if it reformats your + code, that's the final form. Configure (line length, target version) in `pyproject.toml` + `[tool.ruff]`, not via inline `# fmt:` directives. +- **Run `ruff check --fix` before committing.** Most ruff lint rules have safe autofixes, let the + tool handle them. The configured rule families are listed under `[tool.ruff.lint]` `select`. Add + new rule families project-wide rather than scattering inline `# noqa` markers. +- **`# noqa` is a last resort.** When you must use one, scope it narrowly (`# noqa: E501`, not + bare `# noqa`) and add a short comment on the same line explaining why. False-positive patterns + that recur across the codebase belong in `[tool.ruff.lint]` `ignore` or per-file + `[tool.ruff.lint.per-file-ignores]`, with a comment. Porting an existing codebase is not a + license to add `ignore` / `per-file-ignores` blocks to mute newly surfaced lint. Fix it. + +## Comments + +- **Inline `#` comments**: keep tight and local. One line is preferred, but multi-line is fine + when you need to document a non-obvious implementation constraint, a local trade-off, or + coupling that future edits could easily break. Keep that rationale next to the affected block so + the reviewer/maintainer sees it at edit-time. +- **Don't explain what the code does.** Well-named identifiers handle that. Don't reference the + current task ("added for X", "used by Y"), which belongs in the PR description. + +## Docstrings + +- Follow [PEP 257][pep-0257-link]. Focus docstrings primarily on the behavior contract (what + callers and tests can rely on), public semantics, and edge-case expectations. + Implementation-local rationale belongs in inline `#` comments, not docstrings. +- A short one-liner is fine for trivial functions and tests with self-documenting names. +- For non-trivial behavior (non-obvious test scenarios, contracts a test pins, edge cases callers + must know about, design trade-offs that are load-bearing for future maintainers), write a + one-line summary, blank line, then a details paragraph. Multi-paragraph docstrings are fine when + the contract earns it. +- Design notes belong in the code (docstrings or inline comments). They do NOT belong in + `HISTORY.md`, which is end-user release notes, not a design log. + +## Type hints + +- **All public APIs are typed.** The repo's configured type checker runs on `src/` (pyright strict + via `[tool.pyright]` `strict = ["src"]`, or mypy where that is the CI checker), and tests run in + the checker's looser/standard mode. +- **Use modern syntax**: `list[int]` not `List[int]`, `dict[str, X]` not `Dict[str, X]`, + `X | None` not `Optional[X]`, `from __future__ import annotations` only when needed for forward + references. +- **Don't hedge that syntax for an older interpreter.** `pyproject.toml` pins `target-version` / + `python_version` to 3.13 for every Python profile in this repo, and `spec/host-tools.json` + carries that as the host floor `scripts/host_gate.py` enforces, so 3.10+-only syntax (`X | None`, + `match`, etc.) needs no quoting, no `typing.Union` fallback, and no `from __future__ import + annotations` guard on that account alone. Add that import only when a real forward reference + needs it, per the bullet above. Three named exceptions carry a lower floor on purpose and say so + themselves: `scripts/skills_install.sh` and the `install-skills.*` bootstrap scripts, which must + run on whatever interpreter a host already has before this floor's toolchain exists to install + one, and `spec/resolve_description.py`, which `repo-config/configure.sh`'s own bootstrap probe + accepts down to 3.7 for the same reason, and which carries `from __future__ import annotations` + for exactly that purpose rather than out of habit. No other `spec/` code has a reason to hedge, + so that import or a quoted annotation appearing anywhere else in `spec/` is a sign this one + exception got copied rather than a pattern to follow. +- **Don't add `# type: ignore` to silence pyright errors without a comment** explaining the + constraint. If a recurring false positive needs suppression, configure it project-wide in + `[tool.pyright]`. A new port doesn't change this, fix freshly surfaced type errors rather than + muting them. + +## Naming + +- `snake_case` for functions, methods, variables, modules, package directories. +- `PascalCase` for classes, type aliases, type vars, enum members. +- `UPPER_SNAKE_CASE` for module-level constants. +- Single leading underscore for module-private, double leading underscore for name-mangled (rare, + and usually means rethink the design). + +## Imports + +- **Let ruff sort imports.** `[tool.ruff.lint]` `select` includes the `I` rule family + (isort-equivalent). Don't hand-sort. +- Standard library first, then third-party, then first-party (the project itself), each block + separated by a blank line, which ruff enforces automatically. +- Avoid wildcard imports (`from x import *`) outside `__init__.py` re-exports. + +## Patterns to avoid + +- **Don't add backward-compat shims, `# removed` markers, or rename-to-`_` for unused vars**, just + delete. Git history is the audit trail. +- **Don't add error handling for impossible cases.** Trust internal code, and validate only at + boundaries (user input, parsed config, external APIs). +- **Don't use exceptions for expected control flow.** Exceptions are for unexpected states. +- **Don't suppress errors silently** (`except Exception: pass`). Either handle the specific + exception and document why it's safe, or let it propagate. + +<!-- External --> + +[pep-0257-link]: https://peps.python.org/pep-0257/ diff --git a/.github/skills/python-codestyle/references/profiles.md b/.github/skills/python-codestyle/references/profiles.md new file mode 100644 index 0000000..b7eedfb --- /dev/null +++ b/.github/skills/python-codestyle/references/profiles.md @@ -0,0 +1,74 @@ +# Python Profile Details + +## Adapt before propagating + +The rules in `SKILL.md` describe the default Python profile: a package that publishes to PyPI, +type-checked by pyright in strict mode, dependencies in `[dependency-groups]`. A derived repo +often differs, and when it does, adapt these fields to match the repo's actual toolchain rather +than copying verbatim (a verbatim copy that misdescribes the repo is inaccurate and gets rejected +in review). The axes that commonly vary per repo: + +- **Type checker in CI**: pyright strict, mypy in CI with pyright editor-only (Pylance), or both. + Whichever runs in CI is the one the clean-compile and the CI gate invoke. +- **Dependency declaration**: `[dependency-groups]`, or PEP 621 `[project.optional-dependencies]` + (dev tools installed with `uv sync --extra <group>`). +- **Versioning / publishing**: a published package (`_version.py` plus a version source, + `uv build`, and a PyPI publish step), or a source-only repo with a static `version` and no + publish step (see Versioning below). +- **Disabled markdownlint rules**: repo-specific, `.markdownlint-cli2.jsonc` at the repo root is + the source of truth, not any example rule named here. +- **VS Code config home**: editor settings/extensions may live in `.vscode/*.json` or the + `<Repo>.code-workspace`, while tasks/launch/debug configs can only be external `.vscode/*.json` + (they cannot live in the workspace file). The repo's own `tasks.json` sits wherever it keeps it, + and the canonical task definitions it is written against are the hub `vscode-tasks-python.json` + snippet, which resolves the same way from every repo. + +## Two profiles: full specification + +A repo's Python is one of two shapes, declared as the `build` or `lint-only` profile and validated +against the `pyproject.toml` shape. Most of the `SKILL.md` rules (uv project, `uv.lock`, `uv run`, +src layout, pytest coverage) describe the Project shape (the `build` profile). The two differ by +whether the Python has third-party runtime dependencies, which shows up structurally in +`pyproject.toml`, so the fleet's audit reads the shape there: + +- **Project** (the `build` profile): the Python has third-party runtime dependencies, or is the + repo's deliverable. It is a PEP 621 uv project: `[project]` with `dependencies` (dev tools in + `[project.optional-dependencies]` or `[dependency-groups]`), a `[build-system]`, and a committed + `uv.lock` (pinned LF, per GOVERNANCE.md's "Line Endings" section). CI runs `uv sync --frozen` + + `uv run <tool>`, so the lockfile pins tool versions. +- **Scripts** (the `lint-only` profile): stdlib-only utility scripts embedded in a non-Python repo + (e.g. a Python tooling subtree of a `csharp` app). Run the tools with `uvx` (no project install, + no lockfile): the `pyproject.toml` carries only tool config (`[tool.ruff]`, `[tool.mypy]`, and + an optional `[tool.pyright]` editor block), with no `[project]`, no `[build-system]`, and no + `uv.lock` (that metadata would misrepresent it as a shippable package). mypy is the type-check + gate (there is no first-party package for pyright strict to anchor on), and a `[tool.pyright]` + block in standard mode keeps Pylance quiet in the editor, the same mypy-gate/pyright-editor + split the build profile uses. There is no lockfile, and a `uvx <tool>@<ver>` pin in a `run:` + step is not something Dependabot tracks, so CI runs `uvx ruff@latest` / `uvx mypy@latest` rather + than a manual pin that would silently go stale. The fleet rule is to pin only what Dependabot + auto-updates (SHA-pinned actions, package deps) and otherwise run latest, so the VS Code tasks, + README, and CI all run the unpinned latest here. `.py` files follow the repo's LF line-ending + default (per GOVERNANCE.md's "Line Endings" section). There is no pytest suite, and `unittest` is + the runner instead. A script that carries a gate still earns tests, written with the standard + library's `unittest` so they run under bare `python3` with nothing installed, as + `test_<script>.py` under a `tests/` directory beside the scripts it exercises + (`<scripts-dir>/tests/`), kept apart so a test never reads as a tool. Within the scripts + directory the name carries the kind: a gate that checks and exits non-zero on a finding takes a + `_lint` or `_gate` suffix, and a utility that does work takes none. Any repo carrying Python + carries the Python tooling in CI, coverage included, this profile too: `uvx ruff@latest check`, + `uvx ruff@latest format --check`, `uvx mypy@latest`, and the unittest suite under + `uvx coverage@latest run -m unittest discover -s <scripts-dir>/tests` with `coverage report`, + informational with no threshold adopted. A co-present `csharp` type still carries `codecov.yml` + for its own tests. + +## Versioning + +**Published packages.** `_version.py` ships with `__version__ = "0.0.0"` as a placeholder. Until +you wire `_version.py` to something that increments (the usual options are `hatch-vcs`, a +version.json bridge, or manual bumps), no new PyPI versions will land, and publishing with +`skip-existing: true` keeps a stuck placeholder version from failing the run. + +**Source-only repos** (no PyPI publish, with a source-release on dispatch or no release at all) do +not need `_version.py`: keep a static `version` in `pyproject.toml` `[project]`, or let the +release pipeline's version source (e.g. NBGV plus `version.json`) own the tag. There is no publish +step to guard, so `skip-existing` does not apply. diff --git a/.github/skills/python-codestyle/references/testing.md b/.github/skills/python-codestyle/references/testing.md new file mode 100644 index 0000000..0dae3fd --- /dev/null +++ b/.github/skills/python-codestyle/references/testing.md @@ -0,0 +1,17 @@ +# Python Testing Conventions + +This covers the **build** profile. A **lint-only** Scripts profile has no `uv.lock` and does not +use pytest, its testing conventions (`unittest`, `uvx coverage@latest run -m unittest discover`) +are in `references/profiles.md`. + +Use `pytest` with configuration in `[tool.pytest.ini_options]`. Default invocation: +`uv run pytest`. + +- One test file per module under test, named `test_<module>.py`. +- Test functions named `test_<scenario>_<expected_behavior>`, descriptive and not numbered. +- Use fixtures (defined in `conftest.py` for shared ones, or per-test for narrowly-scoped) instead + of setup/teardown methods. +- **Avoid mocking when fakes work.** Hand-rolled fakes that implement the protocol you depend on + are usually clearer and break less than `unittest.mock` magic. +- **Test edge cases that the docstring promises**, not implementation details. If the test breaks + when you refactor without changing behavior, the test is asserting on an implementation detail. diff --git a/.github/skills/repo-worktree/SKILL.md b/.github/skills/repo-worktree/SKILL.md new file mode 100644 index 0000000..9e675f0 --- /dev/null +++ b/.github/skills/repo-worktree/SKILL.md @@ -0,0 +1,229 @@ +--- +name: repo-worktree +description: >- + Mandates and mechanizes task isolation in ptr727/ProjectTemplate fleet repos: every task, + including a continuation of a prior session's task, creates its own git worktree on its own + feature branch before its first file edit, based on the branch work starts on (develop on both + fleet workflow models unless the task is explicitly about main-only content, never whichever + branch a tool defaulted to), preferring a registered worktree in the fleet layout and using a + standalone-clone fallback only when the executor cannot write to either the standard worktree + location or its Git metadata, and no available approval route grants access. Also wraps the + mechanics: + creating a worktree with git worktree add, the fleet layout convention, listing what is in + flight, preparing Husky.Net or Python pre-commit hooks in the new tree, and removing a + worktree and its branch after merge. Use this whenever about to create or edit files in a + fleet repo, whenever starting or resuming a task, whenever the task's branch is already + checked out in a shared checkout, and whenever creating, listing, or removing a worktree. + Triggers even when the session was launched in the primary checkout or the change looks like + a one-line fix, because the primary checkout is the maintainer's own surface and the incident + this guards against was two sessions sharing one checkout, each session's blanket add + committing the other's uncommitted files. +--- + +# Repo Worktree + +## Why This Exists + +Two agent sessions once ran concurrently in the same primary checkout, on the same feature +branch, neither knowing the other was in the tree. One session's commits swept in the other +session's uncommitted files, so two commits landed carrying work their subjects never mention, +committed by a task that never saw it. No rule fired at the moment it was violated, which is the +first file edit: the commit-time and review-time skills all run after a sweep has already +happened. This skill is that missing task-start surface. `GOVERNANCE.md` "Repository Boundaries +and Write Safety" keeps the isolation law and wins on any disagreement, and the mechanics below +are this skill's own content. + +## The Mandate + +- **Every task isolates into its own worktree before its first file edit.** All new work begins + by creating a unique worktree (or clone) on its own feature branch. The primary checkout is + the maintainer's own surface, so a session launched there isolates before writing rather than + after noticing contention. +- **A continuation re-isolates.** A session resuming prior work finds its branch already checked + out somewhere and naturally resumes there, and that instinct is the hazard: a branch sitting + checked out in a shared tree is exactly how two sessions end up in one checkout. Create a + fresh worktree for the continuation and check the branch out there. +- **The moment is the first file edit, not the commit.** By commit time another task's + uncommitted work can already be swept into the staging area, so isolating late protects + nothing. Reading anywhere is fine, and the worktree exists before the first write. +- **Someone else's tree stays theirs.** A branch that changes when nothing you did changed it, + or an edit of yours reverted with no conflict, means another task is live in that tree, and + the response is to stop rather than to re-apply the edit, per `GOVERNANCE.md` "Repository + Boundaries and Write Safety". + +## The Base Branch + +Base the worktree on the branch work starts on for the repository's model, not on whichever +branch a tool defaulted to. GitHub's own "default branch" setting reads `main`, but on both +fleet workflow models work starts on `develop`, so a worktree defaulted to "the default branch" +lands on `main` and silently misses everything merged to `develop` but not yet promoted. Branch +from `develop` unless the task is explicitly about `main`-only content, per `GOVERNANCE.md` +"Branching Model". Fetch immediately before creating and base on the remote ref, because a clone +is whatever it last fetched rather than the branch it names. + +The base clone is a fetch source, not a place to do task work. `fetch` and `worktree add` run +against it for that purpose, and outside "Listing and Cleanup"'s own terminal step below, nothing +else does: never `checkout`, `pull`, `reset`, `commit`, or any other command that mutates its own +working tree, index, or HEAD while a task is in progress. That distinction is the one a real +incident missed, reusing a primary checkout as the working directory itself rather than only as +the source a worktree is created from. On Claude Code this is also a mechanical stop for most +of that list. `merge --ff-only`/`pull --ff-only` and a `checkout <ref>`/`switch <ref>` carrying no +force flag stay exempt even there, matching this skill's own cleanup step below, which needs +exactly those. +Prose remains the only enforcement for a non-Claude-Code agent, and for the shapes the hook itself +exempts. + +## Creating a Worktree + +The fleet layout convention keeps every base clone and every in-flight task visible in one +place: + +```text +~/repos/<Repo> base clone, on its default/working branch +~/repos/worktrees/<Repo>-<task-slug> one worktree per in-flight task, own branch +~/repos/upstream/<owner>-<repo> clone of a repo under another owner, not a fork +``` + +The top level carries no owner segment because everything in it is the fleet owner's own, an +original repo and a fork alike. A fork is named `<upstream-owner>-<upstream-repo>` at fork time, +so a fork of `acme/core` is `acme-core`, and its name identifies the upstream project and stays +unique in the flat namespace without an owner segment of its own. A repository adopted as the +owner's own work rather than kept as a fork is detached from its parent and keeps a plain name, +`widget` rather than `initech-widget`, since it no longer tracks anything upstream. + +A clone of a repository under another owner is neither of those, and flattening one collides +rather than merely reading oddly: `acme/core` joined the way a fork is joined **is** the fork's +name, `acme-core`, while reduced to a bare `core` it names no project and collides with the next +`core` cloned from any other owner. Those clones live one level down under `upstream/`, named by +that same join, so `upstream/acme-core` sits beside the fork it would otherwise land on. The +segment states the relationship rather than the owner, so a reference checkout is told from a +working repo without a `git remote` call, and the names under it never compete with the flat +namespace above. The join is ambiguous in the abstract, since a hyphen in either half means +`acme-labs/core` and `acme/labs-core` produce one name, and it is kept anyway because it is the +fork convention's own join: the ambiguity is inherited from the flat namespace above rather than +introduced here, and it surfaces at clone time as a directory that already exists, where the +second clone takes a hand-picked name. A worktree off one of them keeps the flat worktrees path +under the same name, `~/repos/worktrees/<owner>-<repo>-<task-slug>`. Contributing a change from +such a clone is never a push out of it: fork the upstream first, per the +`upstream-contribution-workflow` skill, and that fork's own clone then belongs in the flat +namespace above, under the name this one already has. + +```sh +git -C ~/repos/<Repo> fetch origin develop +git -C ~/repos/<Repo> worktree add ~/repos/worktrees/<Repo>-<task-slug> -b <task-branch> origin/develop +``` + +The registered worktree above is the normal path. It keeps the task visible in `git worktree +list`, the fleet worktree directory, and IDE worktree discovery. It also leaves the task in a +durable location the maintainer can inspect after the agent session ends. + +Before running that command, inspect the executor's active write boundaries. A linked worktree +requires write access to both of these locations: + +- the intended `~/repos/worktrees/<Repo>-<task-slug>` worktree directory +- the base clone's `.git/worktrees/` administrative directory, which holds the index and locks + +A writable worktree directory does not make the administrative directory writable. When the +executor has an approval mechanism, request scoped approval for the standard `git worktree add` +command. A declared sandbox boundary is the reason to request that approval, not by itself the +reason to skip the registered worktree. + +Use the standard layout when both locations are writable or the executor approves the scoped +write. When approval is unavailable or denied, create a standalone clone under a writable +temporary root. Name it `<temporary-root>/<Repo>-<task-slug>`, fetch immediately, and create the +task branch from `origin/develop`. A standalone clone keeps its worktree and Git administrative +directory under the same writable root. It therefore supports edits, explicit-path staging, +commits, and branch updates without sharing the base clone's index. + +On Claude Code, a standalone clone is structurally a primary checkout to `gh-write-guard.py`'s own +rule 6 test (`--git-dir` equals `--git-common-dir` there too, since it is not a linked worktree of +anything), so the hook denies the very commits/edits this fallback exists to make. Set +`GH_WRITE_GUARD_ALLOW_PRIMARY_CHECKOUT` for the session before using one -- the same escape hatch +`host-setup/agent-safety/README.md`'s requirement 6 already documents -- since this fallback is +exactly the narrow, already-approval-gated case that grant exists for. + +A temporary standalone clone is a degraded handoff, not an equivalent location. The base clone +does not register it, `git worktree list` does not show it, and an IDE opened on the base clone +does not discover its changes. The maintainer must navigate to it manually, and the operating +system may reap it as temporary data. State the absolute path as soon as the fallback is chosen +and again in the handoff. Do not present work there as ordinarily reviewable from the primary +workspace. + +```sh +TASK_ORIGIN="$(git -C <base-clone> remote get-url origin)" +git clone --no-checkout "$TASK_ORIGIN" <temporary-root>/<Repo>-<task-slug> +git -C <temporary-root>/<Repo>-<task-slug> fetch origin develop +git -C <temporary-root>/<Repo>-<task-slug> switch -c <task-branch> origin/develop +``` + +Do not use a linked worktree under the temporary root when the base clone's Git metadata is +read-only. If an existing linked worktree must be kept, index operations require the executor's +scoped approval for that administrative path. Use the standalone clone only after the standard +registered path and its approval route are unavailable. + +A continuation attaches the task's existing branch rather than forking a fresh one: + +```sh +git -C ~/repos/<Repo> fetch origin <task-branch> +git -C ~/repos/<Repo> worktree add ~/repos/worktrees/<Repo>-<task-slug> <task-branch> +``` + +When the base clone holds only the remote-tracking ref, the same command creates the local +branch tracking `origin/<task-branch>` through git's ordinary checkout guessing, so a fresh +clone needs no separate branch setup. Git refuses to attach a branch that is already checked +out somewhere else, and that refusal is the mandate working, since the branch sitting checked +out in a shared tree is the hazard the continuation rule exists for. Return that checkout to +its own working branch first when its tree is clean, and stop when it is not, because a dirty +tree there may be another task's uncommitted work. + +A machine not yet migrated to this layout still isolates exactly the same way, since the mandate +is the isolation rather than the path: create the worktree beside whatever layout the machine +has, and note that the base clone may live elsewhere than `~/repos/<Repo>`. + +## Agent-Specific Worktree Tools + +Provider-specific mechanics stay separate from the general creation procedure above: + +- **Claude Code:** its `EnterWorktree` tool acts only on an explicit instruction from the user or + project instructions. Given a `name`, it creates the worktree under `.claude/worktrees/` and + bases it on the GitHub default branch. Both differ from the fleet path and base. Create the + worktree with `git worktree add`, then attach with `EnterWorktree` `path:`, not `name:`. +- **Codex:** no provider-specific creation override applies. Use the general `git worktree add` + procedure above. Its host-specific writable-root setting lives in `docs/host-setup.md` "Agent + Worktree Access". +- **opencode:** no provider-specific creation override applies. Use the general + `git worktree add` procedure above. + +## Preparing Git Hooks + +A new worktree holds tracked hook configuration but not every generated hook runtime. Prepare +the hooks immediately after creating or attaching the worktree, before the first commit. A +shared `core.hooksPath` value does not make generated files such as `.husky/_/husky.sh` appear +in the new tree. + +- **Husky.Net:** When `.husky/pre-commit` sources `.husky/_/husky.sh` and the local .NET tool + manifest declares Husky.Net, run `dotnet tool restore`, then `dotnet husky install` from the + worktree root. +- **Python pre-commit:** When `.pre-commit-config.yaml` exists, run `uv tool install pre-commit` + once per host if not already installed, then `pre-commit install` from the worktree root. + `pre-commit` is never a project dependency, so this is the same regardless of profile. +- **Repository override:** Follow a repository's explicit hook-setup instructions when they + differ from these standard cases. Do not infer a replacement command from the language alone. + +Treat hook preparation as worktree setup, not as recovery after a rejected commit. If setup +fails, report that boundary and fix the setup. Never bypass the hook to make the commit succeed. + +## Listing and Cleanup + +- `git worktree list`, run in any checkout of a repo, names that repo's base clone and every + worktree with its branch. On the convention layout, one `ls ~/repos/worktrees/` reads what is + in flight across the whole fleet. +- **Cleanup after merge is the default terminal step.** Run it after a squash merge into `develop`. Run it again after a merge-commit promotion into `main`, unless the user explicitly says to retain a checkout or branch. A merge or release handoff is incomplete while finished task, conflict-resolution, installer, or release worktrees remain registered. +- **Verify before removing.** Read the pull request's merged state and head SHA from live GitHub state. Confirm the worktree is clean and resolves to that head. A dirty worktree stops cleanup because force-removing it would discard work. A detached helper worktree needs no pull request, but its commit must be contained in the branch whose completed operation created it. +- **Remove the exact finished worktree, then its local task branch.** Use `git worktree remove <exact-path>`. Try `git branch -d <exact-branch>` after a merge commit. A squash merge does not make the feature tip an ancestor of `develop`, so `-d` cannot recognize it as merged. After the live merged-PR and clean-worktree checks prove that exact branch finished, use `git branch -D <exact-branch>` under the narrow post-squash exception in `git-commit-conventions`. Never apply that exception to an unverified branch or to `develop`. +- **Remove temporary standalone clones and detached helper worktrees too.** Remove the exact `<temporary-root>/<Repo>-<task-slug>` path after confirming it is clean. The remote feature branch follows the repository's normal pull request cleanup policy. Never delete `develop` after a promotion because it is the permanent integration branch. +- **Return the base clone to current `develop`.** Fetch and prune `origin`, confirm the base clone is clean, switch it to `develop` when needed, and fast-forward it with `git merge --ff-only origin/develop`. A completed promotion or release does not leave the base clone on `main`. Stop and report a dirty base clone or a non-fast-forward instead of switching or reconciling it. +- **Prove the cleanup.** Finish with `git status --short --branch` in the base clone and `git worktree list`. The expected result is a clean base clone at `origin/develop` and no worktree belonging only to the completed task. +- A worktree that refuses removal is dirty, and force is not the fix: look at what is + uncommitted in it first, since discarding uncommitted work runs only on explicit instruction, + per the `git-commit-conventions` skill. diff --git a/.github/skills/resync-a-repo/SKILL.md b/.github/skills/resync-a-repo/SKILL.md new file mode 100644 index 0000000..6791899 --- /dev/null +++ b/.github/skills/resync-a-repo/SKILL.md @@ -0,0 +1,85 @@ +--- +name: resync-a-repo +description: >- + Drives RESYNC.md's procedure for bringing a ptr727/ProjectTemplate fleet repo that is already + stood up back into line with the current hub, run from a hub checkout against a named target + repo. Use this whenever asked to resync, sync, converge, or bring a specific repo up to date + with the hub, or to run a conformance sweep against a named repo and apply what it finds. Needs + a hub checkout and a named target repo to mean anything, so it does not usefully trigger from + inside a downstream repo's own session with no target named and no hub checkout present, that + case is fleet-conformance-check instead. Triggers even when the request sounds routine, such as + "just copy AGENTS.md over" or "make repo X match the hub," because that phrasing is exactly how + the AGENTS.md-overwrite incident happened. +--- + +# Resync a Repo + +## Why this exists + +RESYNC.md's own apply order already sequences the remedies so the rules land before the files +they govern and a deletion lands before the re-vendor that would otherwise refresh it. The +AGENTS.md-overwrite incident happened inside that same procedure, on the step that looked most +routine. This skill exists so the mandatory check survives contact with a real, time-pressured +resync instead of depending on an agent remembering to run it unprompted. It is a driver over +RESYNC.md, not a replacement for it. Read RESYNC.md itself for the deletion sweep, the +letters-versus-drift routing, the settings and ruleset step, and everything else that does not +change from one resync to the next. + +## Confirm the procedure before starting + +Read RESYNC.md section 0. A repo with no instruction set at all, or a partial one, is not this +skill's job, it is STANDUP.md sections 1A and 2 instead, since an absent carried file is a +baseline that never arrived rather than drift to converge. Run `spec/audit.py <RepoName>`, the +target's `registry/repos.json` `name` field rather than an `owner/repo` slug or a checkout path, +and read whether the findings are letters (absent) or drift (present but stale) before doing +anything else. The finding kind names the procedure the repo is owed. + +## Reach the hub and measure before changing anything + +Fetch a hub checkout of your own immediately before reading it, per RESYNC.md section 1, since a +stale clone answers confidently instead of failing. Never operate against an existing checkout +already present at a known or shared path, the maintainer's own primary checkout included, even +one that looks current -- always fetch into a private worktree of your own, per `repo-worktree`. +On Claude Code this is now also a mechanical stop for most such commands (a `PreToolUse` hook +denies a mutating git operation run directly in a primary checkout), though this prose is still +the only enforcement for a non-Claude-Code agent, and for the narrow shapes the hook itself +exempts, so following it here is not optional even where no hook can catch a lapse. Verify the +host with `python3 scripts/host_gate.py --repo <path-to-target-checkout>`. Then run the audit end +to end, +RESYNC.md section 2, against the target's `main` branch, never `develop`. A finding is a snapshot, +so quote the run stamp in anything derived from it and re-run before acting on a finding read +earlier in the session. +File any hub defect this work exposes against `ptr727/ProjectTemplate`. +Examples include bugs, conflicting sources, unclear or incomplete instructions, missing capabilities, and Copilot findings about any of them. +Search open and closed issues first, then update the matching issue or file a new one. +Preserve the evidence RESYNC.md section 2 requires, and do not leave the finding only in chat, a review thread, the downstream repo, or agent memory. + +## Apply, in this order + +1. **The instruction set first.** `CLAUDE.md`, then `AGENTS.md` and `GOVERNANCE.md` verbatim sections, then `CODESTYLE.md` and `WORKFLOW.md`, including the `AGENTS.md` skill-dependency pointer paragraph (naming `scripts/skills_install.py` and where the fleet's Skills live) as one more verbatim unit carried in this same step, not a separate pass. `CLAUDE.md` is the single `@AGENTS.md`-import file that gets `AGENTS.md` into a Claude Code session's context at all, a separate baseline entry from `AGENTS.md` itself, so carrying one without the other still leaves that provider unconfigured. **Before touching `AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`, or `WORKFLOW.md` in this step, run the `carried-instruction-file-guard` skill's distinctive-phrase probe against the target file, every time, without exception, regardless of whether the update is a verbatim re-vendor or an intent-fidelity edit.** This is not advisory language to weigh against how routine the diff looks, a diff that looks routine is exactly the shape the AGENTS.md-overwrite incident took. Do not proceed to the re-vendor until the probe has run and any local addition it finds has a destination, per that skill's own procedure. `CLAUDE.md` is outside that guard's scope: it carries no mixed or repo-specific content by design, so its re-vendor is an ordinary verbatim-fidelity copy, no probe needed. +2. **Deletions second, before any re-vendor.** Only a `retire` disposition in + `spec/divergences.json` authorizes removing a file, and the removal is swept tree-wide, per + RESYNC.md section 4, before the deletion counts as done. +3. **Verbatim re-vendors** for `CLAUDE.md` and everything else the probe in step 1 cleared. A + finding classified modified rather than stale gets its diff read before being overwritten, + since it may be an improvement the hub should adopt instead of a mistake to erase. +4. **Interface workflows.** Honor the named contract, required jobs, the ruleset-bound check name, + the artifact-name handoff, rather than copying bytes. +5. **Settings, rulesets, and secrets.** Run + `repo-config/configure.sh check "<owner>/<repo>" release` (substitute `operational` for an + operational repo) from the hub at `main`, then `apply` for what it reports, never from a + carried copy. Run `spec/audit.py [RepoName]` from the same checkout for secrets. +6. **Intent files last, and by hand,** since nothing mechanical judges these. + +Reconcile the registry entry (`status`, `types`, `releaseTrigger`, `workflowModel`, +`driftNotes`) in the same pass, and delete a `driftNote` describing work this pass just finished +rather than leaving it standing. + +## Ship it + +One focused pull request per drift class, branched from the target's `develop`, never a direct +push to a protected branch and never a hand edit outside a pull request. Close the review loop, +per the `pr-review-conduct` skill, before asking the maintainer for merge permission. The +maintainer merges, the agent drives to green and stops. Re-run the audit after the merge and +commit the report once authorized, per `git-commit-conventions`, done means measured, not +applied. diff --git a/.github/skills/shell-codestyle/SKILL.md b/.github/skills/shell-codestyle/SKILL.md new file mode 100644 index 0000000..eaa4a58 --- /dev/null +++ b/.github/skills/shell-codestyle/SKILL.md @@ -0,0 +1,62 @@ +--- +name: shell-codestyle +description: >- + Governs Bash/shell script style for ptr727/ProjectTemplate fleet repos: when a bootstrap or + host-tool script may be shell instead of Python, the mandatory set -Eeuo pipefail header, the + pipefail-versus-early-reader pitfall, self-locating scripts, the shellcheck-plus-shfmt + clean-compile, and the why-not-what comment rule. Use this whenever writing, reviewing, or + editing a shell script (a `.sh` file, or an extensionless bash/sh shebang script), whenever + deciding whether a new script should be Bash or Python, or whenever a pipeline built from + `curl`/`grep`/`jq`-style commands looks like it silently swallowed a failure. Triggers even when + the task looks like a one-line tweak to an existing script, because a missing `-e`/`pipefail`, + or a reader piped straight from a producer that closes the pipe early, are each invisible until + the exact failure mode they guard against actually happens. Fleet-wide: a shell script can + appear in any repo (a bootstrap that installs the interpreter, a host tool that must run before + a toolchain exists), not only a repo whose primary language is shell. +--- + +# Shell Codestyle + +## Why this exists + +This is the shell-specific half of the fleet's code style guide, kept in one place instead of +re-derived per repo or per session. Shell is the fleet's exception language, reached for only +where Python cannot run yet, so its rules exist to keep that narrow surface safe rather than to +cover general scripting style. + +## When shell, not Python + +Bash, and only where a program cannot be Python: a bootstrap that installs the interpreter cannot +be written in it, and a host tool that must run before a development toolchain exists cannot +depend on one. Everything else is Python, with a test under the scripts tree's `tests/` directory. + +## Rules + +- **`shellcheck` is the linter and `shfmt` the formatter.** The clean-compile is `shellcheck` + clean at default severity plus `shfmt -d`, both reporting nothing before a commit. CI enforces + both, per `GOVERNANCE.md` "Running the Linters Locally", and `scripts/docker_lint.py` runs the + same pair headless. Neither is scoped to the `*.sh` glob alone: a tracked, extension-less + script whose shebang names bash or sh (the shape a script meant to run as a bare command takes) + joins the target list too. +- **`set -Eeuo pipefail`, before the first command the script runs.** A header comment sits above + it, as `repo-config/configure.sh` and the `host-setup/` scripts do, since what matters is that + nothing executes unguarded rather than which line number it lands on. Without `-e` a failed + command in the middle of a sequence lets the rest run against a state nobody checked, and + without `pipefail` a pipeline reports the exit of its last stage, so a fetch that failed reads + as an answer when a parser downstream succeeds on an empty input. `-E` carries an `ERR` trap + into functions and command substitutions, so a script that later adds one is not surprised by + where it does not fire. +- **A reader that stops early needs its producer read first.** Under `pipefail`, a producer + writing to a closed pipe exits non-zero, so `curl ... | grep -q` reports a successful fetch as + a failure whenever the match is found early enough. Capture the output, then search it. +- **Self-locating, never dependent on the caller's directory.** A script resolves its own + directory from `BASH_SOURCE` and references its payloads through it, since the working + directory at invocation is not a property of the script. +- **`shellcheck` clean, and a deliberate exception carries its reason inline.** A + `# shellcheck disable=SCxxxx` names why the rule does not apply here, so the next reader can + tell a considered exception from an unread warning. `repo-config/configure.sh` is the worked + example, carrying five `SC2016` disables where a single-quoted `jq` program must stay + unexpanded, each with its reason on the same line. +- **Comments say why, never what.** The code states what it does. A comment restating it goes + stale silently, where a comment carrying a reason fails visibly when the reason stops being + true. diff --git a/.github/skills/skill-lifecycle/SKILL.md b/.github/skills/skill-lifecycle/SKILL.md new file mode 100644 index 0000000..aa85391 --- /dev/null +++ b/.github/skills/skill-lifecycle/SKILL.md @@ -0,0 +1,49 @@ +--- +name: skill-lifecycle +description: >- + Governs the lifecycle of the fleet's own skills in ptr727/ProjectTemplate: creating, changing, splitting, and retiring a skill under .agents/skills/, the source-versus-generated split with .github/skills/ and .claude-plugin/, the regenerate and --check semantics of scripts/build_dist.py, the install and stamp semantics of scripts/skills_install.py, the doc-packaging pattern that keeps a law doc and its skill in agreement, and the trigger-description conventions that make a skill fire. Use this whenever about to create, edit, move, or delete anything under .agents/skills/, .github/skills/, or .claude-plugin/, whenever packaging a doc or a doc section as a skill, and whenever deciding whether a topic deserves a skill at all. Triggers even when the edit looks trivial, such as fixing a typo in one SKILL.md, because the generated distributions desync the moment the source changes without a build_dist.py run, and CI fails the pull request on exactly that. Hub-context only, since .agents/skills/ exists only in the hub. +--- + +# Skill Lifecycle + +## Why This Exists + +The agent most likely to get a skill wrong is the one editing a skill, and before this skill existed nothing watched that moment: the regenerate and install semantics lived in `scripts/` docstrings and scattered prose, so the procedure was rediscovered per session. The two standing hazards are mechanical and silent. A hand-edit to the generated `.claude-plugin/` tree is overwritten by the next regenerate, and a source edit without a regenerate ships a plugin that no longer matches its source, which the CI `--check` gate fails rather than anyone noticing in review. + +## The Pipeline + +- **`.agents/skills/<name>/SKILL.md` is the only hand-authored source**, with optional `references/` and `scripts/` directories beside it. Codex and opencode read this tree directly, project-local, and also read the global `~/.agents/skills/` copy the installer materializes. +- **Generated distributions serve GitHub Copilot and Claude Code.** `scripts/build_dist.py` generates `.github/skills/` for GitHub Copilot and a Claude-plugin-compatible copy at `.claude-plugin/fleet-skills/`, published through `.claude-plugin/marketplace.json`. Neither generated tree is hand-edited, and `build_dist.py --check` exits non-zero when either tree differs from `.agents/skills/`. +- **The skill set is implicit.** Every `.agents/skills/<name>/` directory carrying a `SKILL.md` is a skill, and the generated `plugin.json` derives its list from those directories, so adding or retiring a skill edits no manifest by hand. `marketplace.json` names the plugin, not the skills, and is untouched by ordinary lifecycle work. +- **`scripts/skills_install.py`, run from a hub checkout, installs both forms per machine**: an overlay copy into `~/.agents/skills/` for Codex and opencode, marked per skill so a retired skill is removed on the next run and a foreign skill is never touched, and a user-scope plugin install for Claude Code via the `claude` CLI. Each run stamps the hub commit into `~/.agents/skills-install-stamp.json`, and `--report` reads that stamp against the checkout and exits non-zero when the machine is behind. The install is global per user, and per-repo pinning is a settled non-goal (`docs/fleet-map.md` "Skills Install Model"). + +## Deciding a Topic Deserves a Skill + +A skill surfaces at a trigger moment. A rule that binds every action all the time, or a short reference section a task reads once, gains nothing from being one: the always-on layer is the carried instruction set (`AGENTS.md` and the sections it maps), and packaging it as a skill duplicates it and spends the tokens the delegation rules exist to save. The `AGENTS.md` "Where the Rules Live" map records the disposition either way, a skill annotation on the row or the deliberate absence of one, so a topic with no skill reads as a decision rather than an oversight. + +## Creating a Skill + +1. **Name the directory in kebab-case** and set the frontmatter `name:` to the same string. +2. **Write the `description:` to carry the trigger**, since it is the only part an agent reads before deciding to load the skill: state what the skill governs, then the concrete moments it applies ("Use this whenever..."), then the routine phrasings that precede the failure it guards against ("Triggers even when..."), naming a real incident where one exists. Disambiguate against sibling skills by name, the way `standup-a-repo`, `resync-a-repo`, and `fleet-conformance-check` each state which of the three a session is in. +3. **Author the body per the `comment-and-doc-style` skill**: LF (the repo default), present tense, ASCII tiers, no semicolon in prose. Name hub paths as plain code spans rather than repo-relative links, because an installed copy resolves no repo path, and say "from a hub checkout" for anything the reader must run. +4. **Split bulk into `references/`** when the source doc is large: the SKILL.md carries the summary and the binding rules, and each `references/*.md` carries one topic read on demand, the shape `comment-and-doc-style` uses. +5. **Apply the doc-packaging pattern below in the same change** when the skill packages a law doc or one of its sections. +6. **Regenerate and commit all trees together**: `python3 scripts/build_dist.py`, then, once authorized, commit the source and both generated trees in one commit, per `git-commit-conventions`. CI runs `--check` on every pull request and fails a desynced distribution. `python3 scripts/tests/test_build_dist.py` covers the generator itself. +7. **Record the surfacing**: annotate the `AGENTS.md` "Where the Rules Live" row when the skill packages a GOVERNANCE section, or its closing paragraph when the skill is new content, so the map stays the one place coverage is read from. +8. **Refresh the machines after merge**: re-run `python3 scripts/skills_install.py` per machine, the cadence `docs/host-setup.md` "Fleet Skills Install" states. Until then every machine serves the previous skill set, which `--report` says. + +## Changing or Retiring a Skill + +- **Edit only the source tree.** Any skill-content change under `.github/skills/` or `.claude-plugin/` that did not come from a `build_dist.py` run is a defect, whatever it fixes. +- **Retiring is deleting the source directory and regenerating.** The derived `plugin.json` list shrinks with it, and the installer's per-skill markers remove the retired skill from `~/.agents/skills/` on each machine's next run. +- **A deletion sweeps the prose that references the skill**, in the same change rather than as follow-up: the `AGENTS.md` map row or paragraph naming it, any law-doc packaging pointer to it, and any sibling skill that disambiguates against it. A law-doc section that had moved its full rules into the skill takes them back, or is retired with it, so no rule is silently lost with the skill that carried it. +- **Renaming is a retire plus a create** as far as the installer's markers and the plugin list are concerned, so sweep references the same way. + +## The Doc-Packaging Pattern + +Packaging keeps one topic in one authoritative place while the skill makes it surface automatically. It has two shapes, and each pairing states which it uses: + +- **Moved content.** The law-doc section keeps a summary and the skill holds the full rules (`git-commit-conventions`, `comment-and-doc-style`, `pr-review-conduct`). The section ends with the standard pointer sentence: packaged as the named skill at `.agents/skills/<name>/SKILL.md` in the hub, not a repo-relative link since that path is hub-local and not carried into every fleet repo, read the skill for the full rules. +- **Kept authority.** The source doc keeps the full rules and the skill is the summary that routes to them (`audit-a-repo` over `AUDIT.md`, `workflow-ci-contract` over `WORKFLOW.md`, `agent-conduct` over its GOVERNANCE sections). The skill states per topic which doc section owns it. + +In both shapes the doc wins on any disagreement, and the skill is what needs fixing. A rule stated fully in both places is the drift this pattern exists to prevent, so an edit to a packaged rule lands in its owning place and the other side's summary is checked against it in the same change. diff --git a/.github/skills/standup-a-repo/SKILL.md b/.github/skills/standup-a-repo/SKILL.md new file mode 100644 index 0000000..363d6bb --- /dev/null +++ b/.github/skills/standup-a-repo/SKILL.md @@ -0,0 +1,105 @@ +--- +name: standup-a-repo +description: >- + Drives STANDUP.md's procedure for taking a ptr727/ProjectTemplate fleet repo from nothing (or a + partial state) to operational against the fleet ground truth, run from a hub checkout for a + named target repo the maintainer is standing up. Use this whenever asked to stand up, create, + bootstrap, or onboard a new fleet repo, or to onboard a new repo type. Needs a hub checkout and + a target repo, new or partially started, to mean anything, so it does not usefully trigger + inside an already-operational downstream repo's own session with no hub checkout present, that + case is resync-a-repo for drift or fleet-conformance-check for a self-check instead. Triggers + even when the request sounds like "just copy the template over" or "spin up a quick repo," + because skipping the ordered signing, branch, and instruction-set steps below is exactly how a + repo ends up unsigned, unrecoverable, or authored against unknown rules. +--- + +# Stand Up a Repo + +## Why this exists + +STANDUP.md's own section order exists because several of its steps close a window that cannot be +reopened cheaply: commit signing has to be correct before the first commit, the long-lived +branches have to exist before any standup commit lands on one, and the instruction set has to be +carried before anything else is authored against it. This skill exists so that order survives +contact with a real, time-pressured standup instead of depending on an agent remembering to run +each gate unprompted. It is a driver over STANDUP.md, not a replacement for it. Read STANDUP.md +itself for the full text of every step, the onboarding-a-new-repo-type procedure, and the +cold-start self-test. + +## Before starting + +Read STANDUP.md section 0A first. Nothing in this procedure creates the GitHub repository, its +App, or its secrets, each an outward-facing write that needs the maintainer's explicit permission +and inputs, so hand that checklist over before step 1 rather than discovering the gap partway +through. A repo with no remote is not partially stood up, it is not started, and only the +maintainer can supply what section 0A lists. + +## Apply, in order + +1. **Signing, before the first commit.** STANDUP.md section 0: verify, never set, the inherited + `--global` commit identity and signing configuration, and the host tool floors via + `python3 scripts/host_gate.py`. The window closes at the first commit, since a repo committed + under the wrong identity or unsigned cannot be cleanly repaired afterward. + +2. **Branches, before the first standup commit.** STANDUP.md section 0B: create `main` and + `develop` empty, off one signed empty root commit, then run every step below on a feature + branch off `develop`. Never commit standup work directly onto `develop`. `non_fast_forward` on + both branch payloads, or the missing blocking rule on an operational repo's `develop` ruleset, + makes that mistake either unrecoverable or silently unprotected. + +3. **Classify and catalog.** STANDUP.md section 1: resolve the repo's type(s) against `AUDIT.md` + section 2, then write or repair its `registry/repos.json` entry and confirm it with + `spec/validate.py`. + +4. **The instruction set, before authoring anything.** STANDUP.md section 1A: carry `CLAUDE.md`, + `AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`, `WORKFLOW.md` and `AUDIT.md`, adapted rather + than cloned for the ones that describe a repo, plus `.markdownlint-cli2.jsonc` and + `cspell.json`. `CLAUDE.md` is the fixed, verbatim `@AGENTS.md`-import file that gets + `AGENTS.md` into a Claude Code session's context at all, a separate baseline entry from + `AGENTS.md` itself, so carrying one without the other still leaves that provider unconfigured. + Read `CODESTYLE.md` and the `GOVERNANCE.md` documentation-style rules before writing any repo + content of your own, the same window-closes shape as signing in step 1. + +5. **Capture the source, if one exists.** STANDUP.md section 1B, only when the repo's content + replaces a live external system: capture it and verify the capture against the source before + anything is scaffolded from it, since the source is not under version control and cannot be + re-derived once it stops serving. + +6. **The baseline files.** STANDUP.md section 2: copy every `spec/files.json` entry whose + `appliesTo` matches the repo's selector set, adapted rather than cloned, and choose + `version.json`'s version floor deliberately rather than propagating the template's. Carry + `AGENTS.md`'s skill-dependency pointer paragraph, naming `scripts/skills_install.py` and where + the fleet's Skills live, as one more verbatim unit in this same step, not a separate pass, the + identical requirement `RESYNC.md` places on a repo already stood up. + +7. **The workflows.** STANDUP.md section 3: implement the Actions `WORKFLOW.md` requires for the + repo's type, reusing `catalog/snippets/workflows/` as the reference implementation rather than + inventing a shape. + +8. **Settings, rulesets, and secrets.** STANDUP.md section 4: confirm the remote and the GitHub + repository agree before running anything else here, then run + `repo-config/configure.sh check owner/repo release` (substitute `operational` for an + operational repo) from the hub at `main`. A non-zero exit there means drift was found, not a + command failure. Review what it reports. Then run the same command's `apply` subcommand, which + idempotently reconciles the repo to the full committed configuration regardless of what `check` + reported, never from a hand-built or carried copy. + +9. **Verify with the audit.** STANDUP.md section 5: run `AUDIT.md` end to end. The repo is stood + up only when it passes for its type, or its residual deltas are tracked in + `reports/<repo>/audit.md` plus an issue. + +## Onboarding a new repo type + +When a repo matches no existing type in `spec/project-types.json`, that is a type to onboard, not +a repo to force into the nearest existing one. STANDUP.md's "Onboarding a New Repo Type" section +covers the manifest additions (`spec/project-types.json`, `spec/files.json`, `spec/secrets.json`, +`spec/scope-model.md`, `spec/type-model.md`, and the `registry/repos.schema.json` target enum for +a new publish destination) and the cold-start self-test that proves the result usable by a +context-free agent, not just by the one that wrote it. + +## Ship it + +One pull request per standup, branched from `develop` per step 2 above, into `develop`, never a +direct push to a protected branch. Close the review loop, per the `pr-review-conduct` skill, +before asking the maintainer for merge permission. The maintainer merges, the agent drives to +green and stops. diff --git a/.github/skills/upstream-contribution-workflow/SKILL.md b/.github/skills/upstream-contribution-workflow/SKILL.md new file mode 100644 index 0000000..eec4fcf --- /dev/null +++ b/.github/skills/upstream-contribution-workflow/SKILL.md @@ -0,0 +1,84 @@ +--- +name: upstream-contribution-workflow +description: >- + Governs how the maintainer contributes to a third-party repository he does not control (for + example esphome/esphome), distinct from the fleet's own internal branching model: a dirty work + branch on his own fork for the actual work and review iteration, squashed once clean to a second + branch that carries only the intended minimal history, that clean branch opened as the PR + against the upstream repo, and reviewer feedback applied to the dirty branch first, then + re-squashed into the clean one. Use this whenever about to open a pull request against a + repository outside the ptr727 fleet, whenever forking a third-party project to contribute a fix + or feature, whenever an upstream reviewer requests changes on a PR opened this way, and whenever + deciding which issue or PR template to use for a third-party repository. Triggers regardless of + the target repo's own type or workflow model, since this skill is about the shape of a + contribution to someone else's repo, not the target repo's own internal conventions, which this + skill does not attempt to state and are never assumed to match the fleet's. +--- + +# Upstream Contribution Workflow + +## Why this exists + +The fleet's own branching model (`operational-vs-release-workflow`) governs repos the maintainer +controls end to end: squash-only feature branches, merge-commit promotions, signed commits under +his own identity. None of that applies to someone else's repository. A PR into a third-party +project answers to that project's own maintainers, on their own timeline, with their own review +cycles, and the history that lands there should read as a deliberate, minimal contribution, not as +the maintainer's own iteration log. This skill is that different shape, kept separate from the +fleet's internal model so the two are never conflated. + +## The two-branch shape + +1. **Fork the upstream repo**, if not already forked. +2. **Do the actual work on a dirty work branch**, on the maintainer's own fork. This branch is + allowed to be messy: false starts, fixup commits, back-and-forth in response to review, whatever + the real work looks like while it's happening. Open a PR from this branch into a branch on the + maintainer's **own fork** (not upstream), so all the iteration happens there, visible and + reviewable, without touching the upstream repo at all. +3. **Once the dirty branch is clean and the change is ready, squash it to a second branch** that + carries only the intended, minimal commit history, one commit (or a small, deliberate set) that + states what the change is, not how it was arrived at. +4. **Open the PR against the upstream repo from that second, clean branch.** This is the only + branch upstream ever sees. An upstream draft may be opened only after that clean presentation + branch exists and is published. When more preparation is needed, continue on the dirty branch, + re-squash it into the clean branch, and update the same draft by the step 5 procedure. Never + iterate directly on the published presentation branch. Mark the draft ready when preparation + finishes. Open it ready immediately when no preparation remains. +5. **If upstream reviewers ask for changes, apply them to the dirty branch first**, iterate there + the same way as step 2, then re-squash the updated dirty branch into the clean branch that + actually reaches upstream. Updating the same upstream PR rather than opening a new one each + round rewrites the clean branch's history, and pushing a rewritten branch that is already + published requires `git push --force-with-lease` (prefer it over a bare `--force`, it refuses + the push if the remote moved since the last fetch). **`git-commit-conventions`'s never-force-push + rule governs this fleet's own repos, where a branch is shared with bots, other branches, and + required-check history a rewrite would orphan, and it stays absolute there, with no exception. + It has no jurisdiction here**: this clean presentation branch lives on the maintainer's own + fork, outside the fleet entirely, and carries nobody's work but this squash. Force-with-lease + is scoped just as tightly regardless: only this one branch, only on the maintainer's own fork, + never the dirty work branch, which is the append-only iteration log this whole workflow exists + to preserve. If force-with-lease is ever refused or unavailable, open a fresh PR from a newly + named clean branch rather than fighting the push. + +**The dirty branch is always the working copy. The clean branch is always the presentation copy.** +Never reverse this: never iterate directly on the branch that's open against upstream, and never +skip the squash step because the dirty branch "looks clean enough." + +## Use the upstream repo's own conventions, not the fleet's + +Always use the upstream repo's own issue and PR templates, its own contribution guidelines, and +its own commit-message and code-style conventions when they differ from this fleet's. The fleet's +`comment-and-doc-style`, `git-commit-conventions`, and `pr-review-conduct` skills describe how +*this fleet* does things, and none of them are the target repository's own rules. Read the target +repo's `CONTRIBUTING.md` (or equivalent) and follow it. Where the target repo states no +convention of its own, matching the surrounding code's existing style in that file is the better +default, not falling back to the fleet's own convention by habit. + +## What stays governed by the fleet's own rules + +Signing commits and using the correct git identity are host configuration, not project +convention, so `git-commit-conventions`'s signing and identity rules still apply on both the dirty +and clean branches. They are properties of the committer, not of the target repository. The +write-safety rules (never write to a repository outside explicit authorization, never fabricate a +GitHub id) also still apply in full. A fork the maintainer owns is within scope to push to, and +the upstream repository itself is written to only through the PR the maintainer explicitly asked +for. diff --git a/.github/skills/workflow-ci-contract/SKILL.md b/.github/skills/workflow-ci-contract/SKILL.md new file mode 100644 index 0000000..e02fdfb --- /dev/null +++ b/.github/skills/workflow-ci-contract/SKILL.md @@ -0,0 +1,47 @@ +--- +name: workflow-ci-contract +description: >- + Governs the WORKFLOW.md CI/CD behavioral contract for every ptr727/ProjectTemplate fleet repo: the D1-D9 guarantees stated as the failure mode each prevents, the seam contract for release assets, the artifact lifecycle, NBGV versioning and classification, validate-at-entry, and the 5A/5B/5C test methodology with its per-type walkthroughs. Use this whenever writing or editing anything under .github/workflows/, adding or dropping a release target, auditing a repo's workflows, or reasoning about why a publish did or did not fire. This is the YAML half of the pipeline, and the operational-vs-release-workflow skill keeps the git half (branching, promotion, publish policy), so branch choice questions go there. Triggers even when the edit looks mechanical, such as bumping an action, renaming a job, or adding one upload step, because SHA pinning, the ruleset-bound aggregator name, smoke gating on uploads, and retention-days are each easy to break in a one-line diff that no smoke build exercises, since workflow-only changes are deliberately not smoke-built. WORKFLOW.md keeps authority, and GOVERNANCE.md wins where the two overlap. +--- + +# Workflow CI Contract + +## Why This Exists + +`WORKFLOW.md` in the hub is the largest law doc, a behavioral contract stating required outcomes rather than a required implementation, and it had no skill surface, so agents edited workflow YAML without the contract in view. This skill is the summary plus the binding rules, with the guarantee catalog and the test methodology split into `references/`. `WORKFLOW.md` keeps authority for the contract and methodology, and `GOVERNANCE.md` ("Workflow YAML Conventions", "Release Model") wins where the two overlap. + +## How the Contract Is Read + +- **Outcomes, not bytes.** A workflow is correct when it satisfies the section 4 contract against the expected inputs and outputs, not when it matches a catalog snippet byte for byte. Two repos may implement one guarantee with different YAML. +- **Applicability.** A guarantee governing a construct the repo does not contain is N/A: recorded, excluded from the verdict, never a defect. A source-only pipeline is mostly N/A and that is fine. +- **Operational is binary.** Every applicable guarantee holds, or the workflow is not operational. A single applicable input-output mismatch is a defect regardless of how clean the YAML looks. +- **Reached, not carried.** A standard workflow whose job graph is identical across repos of a type is a `workflow_call` task the hub hosts once, and a repo carries only a caller stub pinned to a hub release commit plus a composite-action hook at `.github/actions/<hook>` for what is its own. A hub task reaches its own actions and sibling tasks through `$/`, which resolves at that pinned commit. The merge-bot is the first, and `docs/reusable-workflows.md` in the hub carries the model, the hook contract, and the phase each workflow migrates in. Until a workflow's phase ships, its copy is graded as below. +- **Two layers.** Orchestration (the PR entry workflow, publisher, version/release/badge jobs) is generic and standard at the job level. Build leaves (`build-<target>-task.yml`) are repo-owned. Inputs like `github`/`nuget`/`dockerhub`/`expect_release_assets` live on the orchestrator, a leaf only receives `ref`/`branch`/`smoke` and a derived `push`, so assert each input in the layer that declares it. What a repo curates is the list of targets, and adding or dropping one edits the whole surface together: the `enable_<target>` input, the `build-<target>` job and its `github-release` `needs:` entry, the `changes` paths-filter entry and output, and the `smoke-build` enable-forward (D6.4). + +## Style Rules That Break in One-Line Diffs + +- **Pin every action to a commit SHA** with a trailing `# vX.Y.Z` comment, first-party included. The one documented no-pin exception is `dotnet/nbgv@master`. Invent no others. +- **Names carry meaning**: `-task.yml` files and "task" names are reusable (`on: workflow_call`), entry points end in what they do and their names end in "action", every job `name:` ends in "job" and every step in "step". A ruleset-bound required check's job `name:` and the ruleset `context:` are one string renamed together, in the live ruleset and the hub's `repo-config/` payloads in lockstep, or required-check enforcement silently breaks. +- **Concurrency**: top-level workflows use `group: '${{ github.workflow }}-${{ github.ref }}'` with `cancel-in-progress: true`. The publisher is the documented exception: a global ref-independent group with `cancel-in-progress: false`, so publishes serialize and never cancel mid-push. +- **Shells**: every multi-line bash `run:` starts `set -Eeuo pipefail`. Multi-line `if:` uses `>-`, never `|`. +- **Boolean inputs** are declared in both trigger blocks and compared against both forms, `${{ inputs.foo == true || inputs.foo == 'true' }}`, since `workflow_dispatch` delivers strings. +- **Permissions validate before `if:`**, so even a skipped job needs valid `permissions:`, and a callee's extra scope (`actions: write`, `id-token: write`) is granted by the caller at the one entry point that needs it. +- **Chaining across optional jobs** allowlists `success`/`skipped` explicitly, because `!= 'failure'` lets `cancelled` through. +- **Docker layer cache** targets a registry tag (`buildcache-<branch>`), never `type=gha`. +- **Workflow YAML is LF.** Preserve endings on every edit. + +## The Core Behavioral Spine + +- **PRs validate fast and never publish**: a paths-filter smoke-builds only changed targets, a type-appropriate validation job always runs, and one required aggregator gates the merge, treating skipped smoke as pass and blocking on failure or cancelled. Smoke does a full compile/lint/test but pushes nothing and uploads nothing, every `upload-artifact` gated `!smoke`. +- **A human merge never auto-publishes**: a `plan` job decides once and every job gates on it. Publishes come from a code-affecting bot push to `main`, a manual dispatch of `main` or `develop`, or the main-only weekly Docker schedule. Each run builds the one trigger branch, `main` a clean `X.Y.Z`, anything else a prerelease `X.Y.Z-g<sha>`, with NBGV owning the patch from git height. The release tags the built commit's SHA (`GitCommitId`), never a branch name. +- **Validate at entry**: cross-input and input-versus-derived-state invariants are asserted once in a dedicated entry job the downstream jobs `needs:`, failing fast with `::error::` before expensive work. The release gate checks branch-versus-prerelease in both directions, strips `+buildmetadata`, and on smoke skips the check while the job still succeeds. +- **The seam contract**: a target contributes a release file by uploading `release-asset-<branch>-<target>`, and the release job collects by `pattern:` plus `merge-multiple:`, never `artifact-ids:`, canonical even for a single target. A repo with no file target passes `expect_release_assets: false` at the caller. +- **Artifacts are an intra-run handoff**: consume-then-delete at the point of consumption, gated to the consumer's condition, best-effort, `retention-days: 1` on every upload as the backstop, and never a blanket delete of the run's artifact set, which destroys the diagnostics you need when the run fails. +- **No-op republish**: an unchanged version re-pushes nothing, the release-create step skips when the tag exists, registries dedupe server-side (`--skip-duplicate`, `skip-existing: true`), and Docker alone always re-pushes by design. +- **A build failure blocks every publish target**: `github-release` needs every build, and the terminal registry pusher guards with `!failure() && !cancelled()`, so nothing partial ships. + +The full catalog, each guarantee with the failure mode it prevents, is in `references/d-guarantees.md`. Auditing, tracing, and probing a repo's workflows is `references/test-methodology.md`. + +## After Any Workflow Edit + +Workflow-only changes are not smoke-built, so run actionlint locally (the Docker invocation in `GOVERNANCE.md` "Running the Linters Locally", which bundles shellcheck for `run:` blocks) before pushing, and remember a workflow change is only fully exercised by CI, since `secrets: inherit`, `permissions:`, and `needs:` wiring resolve only in a real run. diff --git a/.github/skills/workflow-ci-contract/references/d-guarantees.md b/.github/skills/workflow-ci-contract/references/d-guarantees.md new file mode 100644 index 0000000..b41c99a --- /dev/null +++ b/.github/skills/workflow-ci-contract/references/d-guarantees.md @@ -0,0 +1,70 @@ +# The D-Guarantees, Condensed + +Each guarantee is a MUST from `WORKFLOW.md` section 4, stated as input to output plus the failure mode it prevents. This is the condensed catalog for working from, and `WORKFLOW.md` keeps authority, so read the section there when a guarantee's exact wording decides a verdict. + +## D1: PR Fast-Feedback (Smoke) + +- **D1.1** Only changed targets build: each target has a paths-filter entry, unchanged targets skip. Prevents a changed target slipping through unbuilt. +- **D1.2** A validation job always runs on any PR, and a non-.NET repo replaces it (never deletes it), re-pointing every `needs:` on it, the aggregator and `smoke-build` both. Prevents a PR merging with no validation, or a dangling `needs:` failing the workflow to load. +- **D1.3** Smoke never publishes and never uploads: full compile/lint/test, no pushes, every `upload-artifact` gated `!smoke`. Prevents a PR publishing and orphaned artifacts. +- **D1.4** Workflow-file changes are not smoke-built (the filter excludes `.github/workflows/**`), actionlint still validates them. +- **D1.5** One required aggregator gates merge: `needs:` the changes and validation jobs, passes on skipped smoke, blocks on failure or cancelled, and its name is ruleset-bound (job `name:` equals ruleset `context:`, renamed together). +- **D1.6** Coverage reports to Codecov for C# and Python repos with tests, best-effort so an outage never reds the gate, with a `codecov.yml` setting statuses informational and `.gitignore` excluding coverage output. + +## D2: Validation at Entry + +- **D2.1** A dedicated entry job asserts each cross-input invariant before expensive work, downstream jobs `needs:` it. +- **D2.2** The release gate fails loud when the default branch carries a prerelease suffix or a non-default branch carries none, strips `+buildmetadata` first, and on smoke skips the check while the job still succeeds (a job-level `if:` would skip dependents with it). +- **D2.3** A dispatch publish from any ref other than `main` or `develop` fails fast. +- **D2.4** Mutually-exclusive or must-pair inputs are validated, a half-filled combination fails fast. + +## D3: Versioning and Classification + +- **D3.1** One branch per run: `github.ref` names the built branch, NBGV classifies it directly, no `IGNORE_GITHUB_REF`. +- **D3.2** Default branch yields `X.Y.Z`, every other branch `X.Y.Z-g<sha>`, and the default-branch literal in the gate, the `prerelease` expression, and `version.json`'s `publicReleaseRefSpec` all name the repo's real default branch. +- **D3.3** `version.json` sets the major.minor floor, NBGV appends git height as the patch, and both are retained even by a no-compiler repo, since they own the tag. +- **D3.4** Registry versions follow the classification per registry: NuGet.org derives prerelease from the SemVer2 suffix, PyPI builds from `AssemblyFileVersion` with `.dev0` appended on `develop` only, and the develop build stays `--pre`-selectable above the released version. +- **D3.5** A wrapper repo drives its image version from a committed `name -> version` state file, and the leaf must actually read it, since a leaf still tagging off NBGV means the wrapper is not pinned to upstream. + +## D4: Release and Publish + +- **D4.1** Gated single-branch publish: a human merge never auto-publishes, the `plan` job decides once, publishes come from a code-affecting bot push to `main`, a dispatch of `main`/`develop`, or the main-only weekly Docker schedule. +- **D4.2** `target_commitish` is the built commit's SHA (NBGV `GitCommitId`), never a branch name and never `github.sha`. +- **D4.3** Every release is a tag plus source zip, README, and LICENSE, file targets attach `release-asset-*`, and a no-file-target caller passes `expect_release_assets: false` or the release-create step fails on unmatched files. +- **D4.4** No-op republish: an unchanged version re-pushes nothing, the release-create skips when the tag exists (refreshed only on `workflow_dispatch`), registries dedupe server-side, and Docker always re-pushes by design. +- **D4.5** A failed build blocks every publish target: `github-release` needs every build, the terminal registry pusher guards `!failure() && !cancelled()`, so nothing partial ships. +- **D4.6** A deploy check asserts which release and which environment answer, waiting for convergence to a bounded timeout, with an unreachable host reported distinctly from an HTTP status. + +## D5: Resource Cleanup + +- **D5.1** A cross-job transfer artifact is deleted at its point of consumption. An in-run intermediate may rely on the retention backstop. +- **D5.2** The delete runs under the same condition as its consumer, so a no-op re-run skips the release-asset delete while the PyPI build-artifact delete still runs. +- **D5.3** Cleanup is best-effort (`continue-on-error`, tolerate a failed listing, delete all matching ids). +- **D5.4** Every `upload-artifact` sets `retention-days: 1`. +- **D5.5** Never blanket-delete the run's artifacts, which destroys diagnostics and auto-emitted build records. +- **D5.6** A durable deploy destination's retention is bounded by a declared count with one side recorded as owning the prune: the deploy where its credential can observe the destination, the host where the credential is deliberately write-only. + +## D6: Seam Conformance + +- **D6.1** The release job downloads by `pattern:`/`merge-multiple:`, never `artifact-ids:`, canonical for single-target repos too. +- **D6.2** Branch-derived config reads `inputs.branch`, never `github.ref_name`. +- **D6.3** Artifact names are branch-suffixed. +- **D6.4** A target add or drop updates the whole surface together: `enable_<target>` input, `build-<target>` job, `github-release` `needs:` entry, paths-filter entry and output, and the `smoke-build` enable-forward. + +## D7: Concurrency, Permissions, Safety + +- **D7.1** The publisher serializes: global ref-independent concurrency group, `cancel-in-progress: false`. +- **D7.2** Every reusable job declares valid `permissions:` (validated before `if:`), a callee's extra scope granted by the caller. +- **D7.3** Boolean inputs are declared in both trigger blocks and compared against both forms. +- **D7.4** Optional-dependency chaining allowlists `success`/`skipped` explicitly. + +## D8: Bots and Automation + +- **D8.1** The merge-bot enables auto-merge on `opened`/`reopened` for every Dependabot tier, dispatches squash or merge by base ref, disables on a maintainer-pushed `synchronize`, and keys concurrency on the PR number, not `github.ref`. +- **D8.2** Codegen runs a deterministic matrix over both branches, Dependabot targets both branches. +- **D8.3** The upstream tracker writes a committed `name -> version` state file via a rolling per-branch bump PR the merge-bot auto-merges, and its branch prefix must match the merge-bot's head-ref pairs or auto-merge silently never fires. +- **D8.4** An identity allowlist used as a gate emits a `::warning::` on the non-matching branch rather than falling through silently, since a renamed App slug otherwise turns the gate off invisibly. + +## D9: Style and Static + +SHA pins with version comments, the name-suffix rules, `set -Eeuo pipefail`, `if: >-`, registry-tag Docker cache with `cache-to` only the built branch on push and `cache-from` both branches, line endings per `.editorconfig`. diff --git a/.github/skills/workflow-ci-contract/references/test-methodology.md b/.github/skills/workflow-ci-contract/references/test-methodology.md new file mode 100644 index 0000000..ecd0948 --- /dev/null +++ b/.github/skills/workflow-ci-contract/references/test-methodology.md @@ -0,0 +1,27 @@ +# Testing a Repo's Workflows + +The three escalating verification modes from `WORKFLOW.md` section 5, which keeps authority. N/A items (a check or scenario for an absent construct) are recorded and excluded, never failed. + +## 5A: Static Audit + +Read the workflow files plus `version.json` and assert the structural fact behind each applicable D-guarantee, each pass, fail, or N/A with a `file:line` citation, asserting each input in the layer that declares it. The core sweep covers: the paths-filter's target coverage and `.github/workflows/**` exclusion, smoke gating on every upload, the aggregator's `needs:` and skip/fail handling, the entry validation jobs and the two-directional release gate, the single-branch NBGV classification and the three default-branch literals agreeing, `target_commitish` from `GitCommitId`, the consume-then-delete artifact lifecycle with `retention-days: 1` everywhere and no blanket delete, the `pattern:` handoff and `inputs.branch` config, the publisher's serialized concurrency, and the SHA pins. `WORKFLOW.md` 5A lists the per-type addenda (console runtime matrix, NuGet `--skip-duplicate`, the PyPI OIDC environment split, Docker `expect_release_assets` and cache shape, the static-site deploy gates), so apply only the ones the repo's types imply. + +## 5B: Trace Scenarios + +For each applicable scenario, evaluate every job's `if:`/`needs:` against the inputs and compare the predicted run/skip, version, release, and artifact end state to the expected table in `WORKFLOW.md` 5B. The load-bearing ones: + +- **S1** a PR touching a target: that target smoke-builds, nothing uploads, the aggregator succeeds. +- **S5/S6** a bot push to `main`: publishes only when code-affecting, and a human push never does. +- **S7** a publish run builds the one trigger branch with the right classification and leaves no dangling artifacts. +- **S8** a dispatch from a ref other than `main`/`develop` fails fast. +- **S9** a no-op re-run: release-create skipped, registries dedupe, PyPI build artifact still deleted, Docker still re-pushes. +- **S10** branch and version classification disagree: the gate fails loud and everything downstream skips. +- **S12/S13** a deploy dispatch: ref gate first, environment re-asserted, pointer flip separate, live check names the release, and a production deploy from a non-default ref fails before anything is written. + +## 5C: Live Probe + +Only for what a static trace cannot settle: a trivial PR to confirm S1, a smoke push-probe of both branches' version classification, registry queries after a real publish, and the artifact lifecycle read from a real run's logs. The deploy ref gate is verified only by tripping it, and that dispatch is the maintainer's to run: the agent prepares the command and reads back the four evidence items (gate conclusion, its error text, every downstream job skipped, deployment count unchanged), and a harness refusal to fire it is the control working, never something to re-shape. + +## Verdict + +Operational iff every applicable 5A item passes and every applicable 5B scenario matches, with the failing guarantees and their triggering inputs named, and the N/A list recorded. Per-project-type walkthroughs mapping scenarios onto targets, including source-only, static-site, and operational shapes, are `WORKFLOW.md` section 6. diff --git a/.github/workflows/build-docker-task.yml b/.github/workflows/build-docker-task.yml index c66d4dc..18ae2ba 100644 --- a/.github/workflows/build-docker-task.yml +++ b/.github/workflows/build-docker-task.yml @@ -153,7 +153,7 @@ jobs: - name: Create tags and args step id: tagsargs run: | - set -euo pipefail + set -Eeuo pipefail TAGS=$(jq -r '.[]' <<< '${{ toJson(matrix.images.Tags) }}') ARGS=$(jq -r '.[]' <<< '${{ toJson(matrix.images.Args) }}') CACHE_REPO=$(jq -r '.[0]' <<< '${{ toJson(matrix.images.Tags) }}' | sed 's/:[^:]*$//') diff --git a/.github/workflows/merge-bot-pull-request.yml b/.github/workflows/merge-bot-pull-request.yml index 8e9b47b..d8cb6cc 100644 --- a/.github/workflows/merge-bot-pull-request.yml +++ b/.github/workflows/merge-bot-pull-request.yml @@ -53,7 +53,7 @@ jobs: (steps.metadata.outputs.package-ecosystem != 'nuget') || (steps.metadata.outputs.update-type != 'version-update:semver-major') run: | - set -euo pipefail + set -Eeuo pipefail case "${{ github.event.pull_request.base.ref }}" in develop) method=--squash ;; main) method=--merge ;; @@ -97,7 +97,7 @@ jobs: - name: Merge pull request step run: | - set -euo pipefail + set -Eeuo pipefail case "${{ github.event.pull_request.base.ref }}" in develop) method=--squash ;; main) method=--merge ;; diff --git a/.github/workflows/publish-plan-task.yml b/.github/workflows/publish-plan-task.yml index fe493cb..ee1c4fa 100644 --- a/.github/workflows/publish-plan-task.yml +++ b/.github/workflows/publish-plan-task.yml @@ -57,7 +57,7 @@ jobs: ACTOR: ${{ inputs.actor }} REF: ${{ inputs.ref_name }} run: | - set -euo pipefail + set -Eeuo pipefail publish=false case "$EVENT" in workflow_dispatch) diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index fb918cd..850f97a 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -137,7 +137,7 @@ jobs: env: SEMVER2: ${{ needs.get-version.outputs.SemVer2 }} run: | - set -euo pipefail + set -Eeuo pipefail CORE_AND_PRE="${SEMVER2%%+*}" # drop +buildmetadata; a '-' here is the genuine prerelease separator if [[ "$CORE_AND_PRE" == *-* ]]; then echo "::error::Public (main) release version '$SEMVER2' carries a prerelease suffix; refusing to publish." @@ -152,7 +152,7 @@ jobs: GH_TOKEN: ${{ github.token }} TAG: ${{ needs.get-version.outputs.SemVer2 }} run: | - set -euo pipefail + set -Eeuo pipefail if gh release view "$TAG" --repo "$GITHUB_REPOSITORY" >/dev/null 2>&1; then echo "exists=true" >> "$GITHUB_OUTPUT" if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then @@ -205,7 +205,7 @@ jobs: - name: Resolve repository list step id: list run: | - set -euo pipefail + set -Eeuo pipefail # shellcheck disable=SC2016 # the jq program is a literal, not a shell expansion REPOS=$(jq --compact-output \ '[.Images[].Name | ascii_downcase | "ptr727/\(.)"] + ["ptr727/nx-base","ptr727/nx-base-lsio"] | sort | unique' \ diff --git a/.github/workflows/run-codegen-pull-request-task.yml b/.github/workflows/run-codegen-pull-request-task.yml index 64f5916..cac3628 100644 --- a/.github/workflows/run-codegen-pull-request-task.yml +++ b/.github/workflows/run-codegen-pull-request-task.yml @@ -52,13 +52,13 @@ jobs: - name: Run codegen step run: | - set -euo pipefail + set -Eeuo pipefail dotnet run --project ./CreateMatrix/CreateMatrix.csproj -- \ matrix --versionpath=./Make/Version.json --matrixpath=./Make/Matrix.json --updateversion - name: Format code step run: | - set -euo pipefail + set -Eeuo pipefail dotnet tool restore dotnet husky install dotnet csharpier format --log-level=debug . diff --git a/.github/workflows/test-pull-request.yml b/.github/workflows/test-pull-request.yml index 10b7500..342ddea 100644 --- a/.github/workflows/test-pull-request.yml +++ b/.github/workflows/test-pull-request.yml @@ -20,6 +20,10 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +# Deny by default: every job below grants only what its own steps need. +# A job added later inherits nothing rather than the repository default, which is write on this repo. +permissions: {} + jobs: # Inline change-gate (no third-party action): diff the push against its before-commit to decide whether the @@ -30,6 +34,8 @@ jobs: name: Detect image changes job if: ${{ !github.event.deleted }} runs-on: ubuntu-latest + permissions: + contents: read outputs: image: ${{ steps.filter.outputs.image }} base: ${{ steps.filter.outputs.base }} @@ -46,7 +52,7 @@ jobs: BEFORE: ${{ github.event.before }} AFTER: ${{ github.sha }} run: | - set -euo pipefail + set -Eeuo pipefail # A new branch or non-push event has no usable before-commit (all-zeros or empty). Build everything. if [[ "$BEFORE" =~ ^0+$ || -z "$BEFORE" || "${{ github.event_name }}" != "push" ]]; then echo "image=true" >> "$GITHUB_OUTPUT" @@ -74,6 +80,8 @@ jobs: validate: name: Validate job if: ${{ !github.event.deleted }} + permissions: + contents: read uses: ./.github/workflows/validate-task.yml secrets: inherit @@ -88,6 +96,8 @@ jobs: name: Smoke build job needs: [changes, validate] if: ${{ !github.event.deleted && needs.changes.outputs.image == 'true' }} + permissions: + contents: read uses: ./.github/workflows/build-docker-task.yml secrets: inherit with: @@ -105,12 +115,13 @@ jobs: check-workflow-status: name: Check pull request workflow status job runs-on: ubuntu-latest + permissions: {} needs: [changes, validate, smoke-build] if: ${{ always() && !github.event.deleted }} steps: - name: Check workflow results step run: | - set -euo pipefail + set -Eeuo pipefail exit_on_result() { # Pass on success or skipped (smoke-build is skipped when no image files changed). Fail only on # failure/cancelled. diff --git a/.github/workflows/validate-task.yml b/.github/workflows/validate-task.yml index a204f6d..278baac 100644 --- a/.github/workflows/validate-task.yml +++ b/.github/workflows/validate-task.yml @@ -21,6 +21,8 @@ jobs: validate: name: Validate job runs-on: ubuntu-latest + permissions: + contents: read steps: @@ -37,7 +39,7 @@ jobs: # Husky runs the same CSharpier + dotnet format style checks the editor and the pre-commit hook run. - name: Check code style step run: | - set -euo pipefail + set -Eeuo pipefail dotnet tool restore dotnet husky install dotnet husky run diff --git a/.markdownlint-cli2.jsonc b/.markdownlint-cli2.jsonc index d1472b0..ed90f84 100644 --- a/.markdownlint-cli2.jsonc +++ b/.markdownlint-cli2.jsonc @@ -3,9 +3,9 @@ // Prose paragraphs and data-heavy tables or URLs are intentionally long. // Reflowing at 80 columns hurts readability and churns diffs. "MD013": false, - // MD033 (inline HTML) stays enabled so native markdown wins. + // MD033 (inline HTML) stays enabled so native Markdown wins. // HTML comments, used as reference-link dividers, pass it. - // The details and summary elements are allowed for GitHub collapsibles, which have no markdown equivalent. + // The details and summary elements are allowed for GitHub collapsibles, which have no Markdown equivalent. // Every other element still flags. "MD033": { "allowed_elements": ["details", "summary"] }, // Require fenced code blocks over the legacy 4-space-indented style. diff --git a/AGENTS.md b/AGENTS.md index 1a0fb71..5e1f418 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -53,7 +53,7 @@ For comprehensive coding and formatting standards, follow: ### Workspace and linting -- VS Code settings, extension recommendations, and spell-check words live in the workspace file `NxWitness.code-workspace`; add new editor settings or recommended extensions there rather than in `.vscode/`. (Build/debug tasks still live in `.vscode/tasks.json` and `.vscode/launch.json`.) Open the workspace file in VS Code (not the folder) so its settings and recommendations apply. +- VS Code settings and extension recommendations live in the workspace file `NxWitness.code-workspace`; add new editor settings or recommended extensions there rather than in `.vscode/`. Spell-check words live in `cspell.json`, which is the single source of truth for the word list. (Build/debug tasks still live in `.vscode/tasks.json` and `.vscode/launch.json`.) Open the workspace file in VS Code (not the folder) so its settings and recommendations apply. - Linting is editor-only (no CI lint job); the extensions recommended in `NxWitness.code-workspace` cover the project's structured files: C# (Roslyn + CSharpier), Markdown, Dockerfiles/Compose, GitHub Actions workflows, and spelling. Lint changed files and clear reported problems before pushing. - For workflow files, the GitHub Actions extension covers schema/expression checks in-editor; run the `actionlint` CLI for deeper checks (including shellcheck on `run:` steps). diff --git a/NxWitness.code-workspace b/NxWitness.code-workspace index b9553ab..beb4d11 100644 --- a/NxWitness.code-workspace +++ b/NxWitness.code-workspace @@ -5,105 +5,6 @@ } ], "settings": { - "cSpell.words": [ - "accessibilities", - "adduser", - "Allman", - "apikey", - "appdata", - "autoremove", - "btrfs", - "BUILDPLATFORM", - "buildtransitive", - "buildx", - "cloudhost", - "codegen", - "Composefile", - "connrefused", - "contentfiles", - "csdevkit", - "davidanson", - "digitalwatchdog", - "dockerdirectory", - "dockerenv", - "Dockerfiles", - "domainname", - "dotnettools", - "dwspectrum", - "esac", - "filesystems", - "getent", - "getmatrix", - "gruntfuggly", - "Hanwha", - "hddpool", - "IPVMS", - "kinnairdclan", - "lsio", - "lsiobase", - "macvlan", - "makedirectory", - "matrixpath", - "matrixschemapath", - "mediaserver", - "metavms", - "Moby", - "nameof", - "networkoptix", - "Newtonsoft", - "noatime", - "nodev", - "noninteractive", - "NONROOT", - "NOPASSWD", - "nosuid", - "nvrpool", - "nxgo", - "NXMETA", - "nxvms", - "NXWITNESS", - "Optix", - "PGID", - "Pieter", - "pipefail", - "posixacl", - "PUID", - "relatime", - "repoint", - "repointed", - "Repot", - "resharper", - "schemamatrix", - "schemaversion", - "seccomp", - "Serilog", - "shfs", - "Signup", - "slnx", - "ssdpool", - "subvol", - "subvolid", - "tagsargs", - "TARGETARCH", - "TARGETPLATFORM", - "tmpfs", - "traefik", - "trunc", - "Unraid", - "updateversion", - "usermod", - "versionlabel", - "versionpath", - "versionschemapath", - "Viljoen", - "whitelabel", - "Wisenet", - "wisenetwave", - "WORKDIR", - "xattr", - "xunit", - "yzhang" - ], "files.trimTrailingWhitespace": true, "files.trimTrailingWhitespaceInRegexAndStrings": false, "diffEditor.ignoreTrimWhitespace": false, diff --git a/WORKFLOW.md b/WORKFLOW.md index 5aa1a35..63bb1bd 100644 --- a/WORKFLOW.md +++ b/WORKFLOW.md @@ -92,7 +92,7 @@ Legibility rules. Necessary but not sufficient: a perfectly styled workflow can it: a ref-independent group with `cancel-in-progress: false`, so two publishes never overlap (a schedule, a dispatch, and a `Make/Matrix.json` pin push, or back-to-back dispatches against the shared Docker tags) and none is cancelled mid-release. Codegen keys on the workflow only (it writes fixed `codegen-*` branches). -- **Shells.** Every multi-line bash `run:` starts with `set -euo pipefail`. +- **Shells.** Every multi-line bash `run:` starts with `set -Eeuo pipefail`. - **Conditionals.** Multi-line `if:` uses the folded scalar `if: >-`. - **Boolean inputs.** A boolean used by both `workflow_call` and `workflow_dispatch` is declared in both trigger blocks and compared against `true` and `'true'`. @@ -500,7 +500,7 @@ Each is a **MUST**, stated as input -> output plus the failure it prevents. track latest. - **D9.2** File/workflow/job/step names follow the suffix rules; a ruleset-bound `context:` name moves only in lockstep with `repo-config/`. -- **D9.3** Bash `run:` blocks start `set -euo pipefail`; multi-line `if:` uses `>-`. +- **D9.3** Bash `run:` blocks start `set -Eeuo pipefail`; multi-line `if:` uses `>-`. - **D9.4** Line endings follow `.editorconfig`. - **D9.5 No decorative / dropped workflows.** No date-badge (`build-datebadge-*`), no standalone docker-readme task (folded into the publisher), no `PUBLISH_ON_MERGE` variable, no `dorny/paths-filter` (replaced by the diff --git a/cspell.json b/cspell.json index aab8007..32192dc 100644 --- a/cspell.json +++ b/cspell.json @@ -11,8 +11,10 @@ ], "words": [ "accessibilities", + "adduser", "Allman", "apikey", + "appdata", "arahata", "astral", "autoremove", @@ -20,14 +22,20 @@ "buildcache", "buildcaches", "buildmetadata", + "BUILDPLATFORM", "buildtransitive", + "buildx", "Buildx", + "bumpable", "charliermarsh", "chowned", "chowns", + "cloudhost", "codegen", "commitish", + "Composefile", "conftest", + "connrefused", "contentfiles", "cref", "crefs", @@ -35,31 +43,43 @@ "datebadge", "davidanson", "debuglevel", + "desync", "devcontainer", "diffable", "digitalwatchdog", "distros", + "dnsmasq", "dockerbuild", "dockerdirectory", + "dockerenv", "Dockerfiles", "dockerhub", "Docstrings", "dogfoods", + "domainname", "dorny", "dotnettools", "downstreams", + "dpkg", "dryrun", "dwspectrum", "Emby", "envsubst", + "esac", + "esphome", "extensionless", "fanaticpythoner", + "filesystems", "finalizers", "Genericize", + "getent", + "getmatrix", "gpgsign", + "gpgv", "gruntfuggly", "HACS", "Hanwha", + "hass", "hatchling", "hddpool", "heredocs", @@ -71,8 +91,11 @@ "isort", "Jellyfin", "Keychain", + "keyring", + "keyrings", "kicad", "kinnairdclan", + "koalaman", "lastbuild", "libsecret", "LINQ", @@ -82,10 +105,12 @@ "macvlan", "makedirectory", "matrixpath", + "matrixschemapath", "mediaserver", "metavms", "mktemp", "Moby", + "mstruebing", "mypy", "nameof", "nbgv", @@ -96,7 +121,9 @@ "noatime", "nodev", "noninteractive", + "NONROOT", "nonroot", + "NOPASSWD", "noqa", "nosuid", "nuget", @@ -104,9 +131,13 @@ "nvrpool", "nxgo", "nxmeta", + "NXMETA", "nxvms", + "NXWITNESS", "nxwitness", "onCreateCommand", + "opencode", + "opencontainers", "Optix", "othercommand", "paramref", @@ -127,14 +158,22 @@ "pyright", "pyrightconfig", "pytest", + "Qodo", "quoteoftheday", + "regen", "reimplementation", "relatime", + "repoint", + "repointed", "Repot", "resharper", + "resync", + "resynced", "rhysd", "Rubba", "ruff", + "schemamatrix", + "schemaversion", "sdist", "seccomp", "semver", @@ -143,33 +182,46 @@ "shellcheck", "shfs", "signingkey", + "Signup", "slnx", "snupkg", "softprops", "somecommand", "ssdpool", "sshconfig", + "stdlib", "subsetting", "subvol", "subvolid", + "tagsargs", + "TARGETARCH", + "TARGETPLATFORM", "timonwong", "tmpfs", + "traefik", "Triaging", "trunc", "tzdata", "unbuilt", + "unconfigured", "Unraid", + "untriaged", "unvalidated", "updateversion", + "usermod", "USERPROFILE", + "uvx", "venv", "versionlabel", "versionpath", + "versionschemapath", "Viljoen", "whitelabel", "winget", "Wisenet", "wisenetwave", + "WORKDIR", + "worktree", "xattr", "Xsession", "xunit", diff --git a/host-tools.json b/host-tools.json new file mode 100644 index 0000000..48d9d87 --- /dev/null +++ b/host-tools.json @@ -0,0 +1,4 @@ +{ + "note": "This repository's own host-tool declaration, layered over the fleet declaration in the hub's spec/host-tools.json by scripts/host_gate.py. Layering is tighten-only: an entry here may add a tool, raise a floor, or turn an optional tool required, and may not lower a floor or turn a required tool optional, because that would retire a fleet check from inside the repository it protects. The tools list is empty because this repository needs no tool the fleet declaration does not already carry, and the file is still present rather than absent so the declaration is somewhere a reader can find rather than somewhere they have to know to look.", + "tools": [] +}