Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ indent_size = 2
[.github/workflows/*.{yml,yaml}]
end_of_line = lf

# Composite action metadata is Actions-owned like the workflows above, and Dependabot rewrites the
# `uses:` pins inside it with LF exactly as it does theirs, so declaring LF keeps it consistent.
[.github/actions/**/*.{yml,yaml}]
end_of_line = lf

# Catalog snippet workflows mirror the real workflow files pinned LF above.
# Keep the snippets LF, so a copied snippet lands compliant instead of needing conversion.
[catalog/snippets/workflows/*.{yml,yaml}]
Expand Down
97 changes: 97 additions & 0 deletions .github/actions/prose-gate/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# Fleet prose gate, consumed by downstream repos so the rules live here rather than in 20 copies.
# Pin this action to a commit SHA per GOVERNANCE.md "Action pinning".
# The pin is one literal, identical on both branches.
# `uses:` takes no expressions, so a per-branch ref would diverge at every promotion.
# Branch-dependent behavior therefore lives here, where expressions are legal.
name: Fleet prose gate
description: Check the lines a change touches against the fleet prose rules.

inputs:
base:
description: Ref to diff against, so only lines the change touches are reported.
required: true
paths:
description: Paths to scan.
required: false
default: .
rules-ref:
description: >-
Hub ref supplying the rules.
Empty selects develop on every branch except main, which takes the pinned action version.
required: false
default: ''

runs:
using: composite
steps:
# A develop-targeted run reads the rules from hub develop.
# That exercises an unpromoted rule change fleet-wide before it reaches main.
# Every other run uses the copy bundled at this action's pinned SHA.
# A released repo's gate is then reproducible, and a hub commit cannot fail a re-run.
- name: Resolve the rules source step
id: rules
shell: bash
env:
REQUESTED: ${{ inputs.rules-ref }}
# `base_ref` is set only on a pull_request event, and a fleet repo gates on push instead.
# The branch name is the reliable signal, so it is read whenever `base_ref` is empty.
TARGET: ${{ github.base_ref || github.ref_name }}
run: |
set -Eeuo pipefail
ref="${REQUESTED:-}"
# Only main is pinned, since it is the released tier and a re-run of it must not change.
# Every other branch merges into develop, so it tracks develop and exercises rules early.
if [ -z "$ref" ] && [ "$TARGET" != "main" ]; then ref="develop"; fi
if [ -z "$ref" ]; then
# Three levels up from the action directory is the repository root of this pinned checkout.
bundled="$GITHUB_ACTION_PATH/../../../scripts/prose_lint.py"
if [ ! -f "$bundled" ]; then
echo "::error::Bundled prose_lint.py not found at $bundled" >&2
exit 1
fi
echo "script=$bundled" >>"$GITHUB_OUTPUT"
echo "Rules source: bundled at this action's pinned version"
else
dst="$RUNNER_TEMP/prose_lint.py"
url="https://raw.githubusercontent.com/ptr727/ProjectTemplate/$ref/scripts/prose_lint.py"
# Retry a transient network failure, since it would otherwise fail a correct change.
# A non-200 still fails the step, because -f keeps the trust model unchanged.
# Fail loudly rather than skipping the gate, since a silent skip reports a clean pass.
curl -fsSL --retry 3 --retry-delay 2 --retry-all-errors "$url" -o "$dst"
echo "script=$dst" >>"$GITHUB_OUTPUT"
echo "Rules source: hub $ref"
fi

# The caller checks out with fetch-depth 0, because a diff against the base needs its history.
- name: Check prose step
shell: bash
env:
SCRIPT: ${{ steps.rules.outputs.script }}
BASE: ${{ inputs.base }}
PATHS: ${{ inputs.paths }}
run: |
set -Eeuo pipefail
# Check the base resolves before scanning, so an empty or absent ref fails naming itself.
# Unresolvable, the run would report the repository's whole backlog against this change.
# A shallow checkout is the usual cause, so the caller fetches full history.
if ! git rev-parse --verify --quiet "$BASE^{commit}" >/dev/null; then
echo "::error::Diff base '$BASE' does not resolve in this checkout." >&2
echo "::error::Check the ref name and that the job checks out with fetch-depth 0." >&2
exit 1
fi
# A caller may write paths as a multi-line YAML block, and `read -ra` stops at a newline.
# Fold newlines into spaces first, so every path is read.
# Left unfolded it scans only the first path and reports the rest clean.
# That is a silent under-scan, the one failure a gate must never have.
# Splitting on whitespace keeps several paths as several arguments.
# Passing the array quoted keeps each element literal, where an unquoted one would glob.
read -ra scan <<<"$(printf '%s' "$PATHS" | tr '\n' ' ')"
if [ "${#scan[@]}" -eq 0 ]; then
echo "::error::No paths to scan after parsing the paths input." >&2
exit 1
fi
echo "Scanning ${#scan[@]} path(s): ${scan[*]}"
# Options first, then `--`, so every remaining token is read as a literal path.
# A path-shaped token beginning with a dash would otherwise parse as an option, and
# `--list-files` in that position turns the gate into a file listing that exits 0.
python3 "$SCRIPT" --diff "$BASE" -- "${scan[@]}"
11 changes: 9 additions & 2 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,17 @@ Auto-review on push is configured (via the branch ruleset's `copilot_code_review
gh api repos/<owner>/<repo>/pulls/<N>/reviews --jq \
'.[] | select(.body | test("Suppressed comments|low confidence")) | .body'

# Scope it to the current head, so an answered finding from an earlier round does not re-open.
# Read every round, not only the head. A suppressed finding has no resolved state, so a push
# does not retire it: it simply stops appearing in a head-scoped query while still unanswered.
# Head-scoping this query is how four rounds went unanswered across three pull requests in a day.
gh api repos/<owner>/<repo>/pulls/<N>/reviews --jq \
'[.[] | select(.body | test("Suppressed comments|low confidence"))] | length'

# Mark which round each came from, since a finding on an older round may since be moot.
PR_HEAD=$(gh pr view <N> --json headRefOid --jq '.headRefOid')
gh api repos/<owner>/<repo>/pulls/<N>/reviews --jq \
"[.[] | select(.commit_id==\"$PR_HEAD\") | select(.body | test(\"Suppressed comments|low confidence\"))] | length"
"[.[] | select(.body | test(\"Suppressed comments|low confidence\"))
| {round: (if .commit_id == \"$PR_HEAD\" then \"head\" else \"earlier\" end), id}]"
```

**Round 1 is normally auto-seeded, so poll for it before trying to self-trigger.** Auto-review-on-open supplies the first review with no `botIds` call needed, but it can lag one to three minutes. After opening a PR (or the first push), **poll** for a Copilot review on the head SHA (see [Verify Review Covered Current Head](#verify-review-covered-current-head)) before concluding none ran. The `requestReviews` mutation below is for **re-requesting on later pushes** (a new head SHA). By then a prior review exists, so its bot node id is readable. A missing bot node id on round 1 therefore means "the auto-review has not landed yet - wait and poll," **not** "ask the maintainer to kick it off."
Expand Down
11 changes: 10 additions & 1 deletion GOVERNANCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ A state-changing GitHub call is the highest-blast-radius thing an agent does her
## Git and Commit Rules

- **Default to staging, not committing.** Stage changes with `git add` and leave `git commit` to the developer unless the developer has explicitly authorized the agent to commit for the current ask ("commit this", "open a PR", etc.). Authorization is scope-bound: it covers the commits needed for that specific task, not a blanket commit license for the rest of the session.
- **"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 work that stops 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 (see the Branching Model), and never with `--force`. Holding a commit locally is the narrower case, so it happens when the developer asks for it rather than by default.
- **Check the working tree for the maintainer's own uncommitted edits before committing.** The maintainer hand-edits files live (often `README.md`/`HISTORY.md`, sometimes with the editor's LF->CRLF flip on top). Review `git status` first. If there are changes you did not make, ask whether to include them rather than bundling half-finished work or stranding it in an unrelated commit.
- **All commits must be cryptographically signed (SSH or GPG).** Branch protection enforces this on both branches, and unsigned commits are rejected on push. Signing depends on environment configuration: `git config commit.gpgsign true`, a configured `user.signingkey`, and a working signing agent (loaded `ssh-agent` for SSH, or `gpg-agent` for GPG). If signing is not configured in the environment, **do not commit**. Surface the missing config to the developer and stop at `git add`. Verify before any agent-authored commit (`git config --get commit.gpgsign && ssh-add -L` or the GPG equivalent). **Signing must be live before the *first* commit, not retrofitted.** Turning on `Require signed commits` against a branch that already has 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 (a rebase preserves the `author` field but not the original signatures, and you cannot sign another contributor's commits for them). During new-repo setup, never create commits until signing is verified.
- **Commit under the committing account's own GitHub `noreply` identity, never a private, personal, or invented address.** The `author` and `committer` on every agent-authored commit are the GitHub `noreply` address of the account whose key signs the commit (above). GitHub issues these in a `username@users.noreply.github.com` or `ID+username@users.noreply.github.com` form, and for this single-maintainer fleet it is the owner's `ptr727@users.noreply.github.com`. Do not set `user.name`/`user.email` to a fabricated persona, bot name, or product name, and do not commit under whatever identity the environment happens to carry: verify `git config --get user.email` is that GitHub `noreply` address before committing. **Verify it, do not set it.** The identity is host configuration, set globally once, so a repo-local `user.email` is redundant where the global is right and a wrong identity where it is not, and it silently shadows the global it overrides. A mismatch is a host fault to surface to the maintainer rather than to patch per repo, because a local override hides a broken host that then commits under the wrong identity in every other repo on that machine. A wrong identity is not cosmetic: a private email trips GitHub's email-privacy push protection (GH007), and an unrecognized or invented author pollutes history. Identity is separate from signing: a wrong author does not by itself fail the signature rule, but the ad-hoc identities that produce it are typically also unsigned, which the signing rule above then rejects on push.
Expand Down Expand Up @@ -258,6 +259,8 @@ For provider-specific mechanics (how to request review, query review state, post

### Triaging Review Comments

**A low-confidence finding is not a low-value one.** Copilot collapses the findings it is least sure of into the review body instead of raising a thread, and in this fleet's experience those are right the large majority of the time. Judge each one against the code, never against its confidence label. They are also the easiest to lose, because they appear in no thread, so a loop that polls threads alone reports a clean pass while they stand (see the Merge Gate, condition 3).

For each comment, classify before responding:

- **Bug** - wrong behavior, missing test coverage, or a real divergence between code and docs. Fix it. Reply with the fixing commit SHA when done.
Expand All @@ -268,10 +271,16 @@ For each comment, classify before responding:

### Responding and Resolution Expectations

Reply inline with either the fixing commit SHA (for accepted issues) or a concise rationale (for declines). Resolve review threads when addressed or intentionally declined with rationale. Issue-level comments (those at `repos/.../issues/<N>/comments` rather than tied to a specific line) have no resolution action, so acknowledge with a reply if needed and move on.
Reply inline with either the fixing commit SHA (for accepted issues) or the evidence that disproves it (for declines). **A decline carries proof rather than an assertion**, meaning the command and its output, the code path that makes the concern impossible, or the rule that governs it. "This is fine" is not a reply, and disagreeing without evidence is not addressing a finding, so a thread is not resolved on one. Resolve review threads when addressed, or when declined with that evidence recorded in the thread. Issue-level comments (those at `repos/.../issues/<N>/comments` rather than tied to a specific line) have no resolution action, so acknowledge with a reply if needed and move on.

After the final push on a PR, sweep older threads from earlier rounds whose code paths no longer exist, or stale unresolved markers remain in the review UI.

**Answering a suppressed finding is a different act from replying in a thread, and it carries its own pairing.** A threaded reply sits under the comment it answers and the UI shows whether it is resolved. A suppressed finding has neither, so an answer that does not carry its own context is unverifiable: the maintainer cannot tell that it was seen, which finding it addresses, or whether any were skipped, and has to ask. An answer therefore **quotes the finding** in a blockquote, with its `file:line` anchor and enough of Copilot's own words to identify it, **carries one bold verdict per finding** (`Fixed in <SHA>`, `Disproven`, or `No change needed`) so the outcomes are scannable without reading prose, **states the `(N)` count** the block heading gives so N answers can be checked against N findings, and **links the review** that raised them, since a PR accumulates rounds and an unlinked answer is ambiguous about which one it closes. One comment per review round keeps the answers together.

**Read every round, not only the head.** A suppressed finding has no resolved state, so a push does not retire it: the finding simply stops appearing in a head-scoped query while remaining unanswered. Treating "superseded by a push" as "answered" is how rounds of findings go unanswered. `scripts/pr_review.py status <n>` reports every round and marks which are from earlier ones.

**The review's own overview cannot be trusted to say whether findings exist.** A body that reads "Copilot reviewed N out of N changed files and generated no new comments" routinely carries a collapsed block of suppressed findings directly beneath that sentence. Read the body for the block rather than the summary line, because the summary line and `reviewDecision` and an empty unresolved-thread list all agree that a review with four outstanding findings is clean.

### Escalating to the User

Bring the user in when:
Expand Down
3 changes: 3 additions & 0 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@ Running backlog for this repo, kept in a committed file so the guidance survives
- Decide whether the canonical README section order follows PlexCleaner, which is a separate question from the four divergences above and affects every repo plus the `readme-structure` audit. PlexCleaner places **Questions or Issues** immediately after the Table of Contents, where the spec orders it ninth, and it carries sections the spec names nowhere, including Performance Considerations, Runtime Metrics, Custom Plugins, Testing, Development Tooling, Feature Ideas, and Sample Media Files. Under the recurrence rule in [`spec/section-model.md`][section-model] those last ones are correctly repo-specific and stay undeclared, so the open question is only the position of the sections the spec already names.
- Declare locally-required secrets the way GitHub-stored ones are already declared, and make a gitignored `secrets/` directory the fleet standard that holds them. [`spec/secrets.json`][secrets] covers only the Actions and Dependabot stores, so a repo that deploys somewhere has no declared way to say what it needs at runtime, and the required set is discoverable only by reading the deploy. The pattern already runs in the fleet in two shapes: HomeAutomation-Config keeps a gitignored secrets directory of env files and Docker secret files, and ESPHome-Config keeps a gitignored `secrets.yaml` beside a committed `_secrets.yaml`. The committed file carries the required names with dummy values, so the shape of the requirement is in git while the values never are, which is the same split the GitHub side already gets from `requiredSecrets[]`. Blog needs it immediately, since it deploys on the proxmox host through HomeAutomation-Config's Docker Compose stack and carries the copy destinations and the internal URI. The hub carries neither the directory nor a `.gitignore` entry for one today, so adopting it here comes first.
- Re-vendor `repo-config/configure.sh` across the fleet. The hub swept it to one sentence per line, and it is carried `verbatim` with `appliesTo: "*"`, so every repo already holding a copy is byte-mismatched against the hub until it takes the new one.
- Investigate replacing copy-pasted workflow content with cross-repo reuse, now that this repo is public. A public repository's composite actions and reusable workflows can be consumed by any other repository regardless of owner type, so the organization account this pattern was assumed to require is not needed, and the constraint that shaped the current vendor-everything model no longer holds. The catalog under [`catalog/snippets/workflows/`][workflows] is copied into each repo today, which means a fix to a shared job is a sweep across the fleet rather than one edit here, and it is the mechanism by which a defect in a snippet seeds itself into every repo that adopted it. Scope the investigation to which jobs are genuinely identical across repos against which only look similar, since a reusable workflow that needs a long input list to cover per-repo variation is worse than the copy it replaces. Settle the ref policy in the same pass, because consuming hub code at CI time is a floating dependency unless it is pinned, and [`GOVERNANCE.md`][governance] "Action pinning" requires a commit SHA for every action with one documented exception. Note that `uses:` does not accept expressions, so a per-branch ref cannot be selected in the workflow file and any branch-dependent behavior belongs inside the consumed action instead.

<!-- Repo -->

[audit]: ./spec/audit.py
[audit-doc]: ./AUDIT.md
[governance]: ./GOVERNANCE.md
[matrix]: ./reports/conformance-matrix.md
[readme-structure]: ./spec/readme-structure.md
[reports]: ./reports/
[repos]: ./registry/repos.json
[secrets]: ./spec/secrets.json
[section-model]: ./spec/section-model.md
[standup]: ./STANDUP.md
[workflows]: ./catalog/snippets/workflows/
Loading
Loading