Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
146 changes: 136 additions & 10 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ For full rationale see [`AGENTS.md`](../AGENTS.md). Quick rules:

- `feature → develop → main`. PRs only.
- Develop accepts **squash merges only**; main accepts **merge commits only**. Don't suggest rebase-merge — it's disabled at the repo level.
- Both branches **auto-publish on push**: develop produces NBGV prereleases (`X.Y.Z-g{sha}`) tagged `develop` on Docker Hub; main produces stable releases (`X.Y.Z`) tagged `latest`.
- **Two-phase publishing.** PRs only **smoke-build** changed targets (Docker `linux/amd64`, a 2-runtime executable subset, no push). `publish-release.yml` is the sole publisher: its **weekly schedule + manual dispatch** build/publish **both** branches (develop NBGV prereleases `X.Y.Z-g{sha}` tagged `develop`; main stable `X.Y.Z` tagged `latest`). Routine merges do **not** publish unless the `PUBLISH_ON_MERGE` repo variable is `true`.
- Dependabot targets **both** `main` and `develop` with the same ecosystems; major NuGet bumps gate on human review, everything else auto-merges via App-token-driven merge-bot.
- Every third-party GitHub Action is pinned to a full commit SHA with a `# vX.Y.Z` comment. Don't introduce `@v6` / `@main` / `@master` floating refs.
- Never merge a PR without a fresh "no issues found" review from `copilot-pull-request-reviewer[bot]` (shown as "Copilot" in the UI) on the latest commit. `mergeStateStatus: CLEAN` is necessary but not sufficient — Copilot's re-review of the latest push is required. Re-request the review **programmatically** after every push via the `requestReviews` GraphQL mutation (don't wait on flaky auto-review-on-push) — see the [GitHub Copilot Review Runbook](#github-copilot-review-runbook) below and [`AGENTS.md`](../AGENTS.md#merging-a-pr).
- After a develop → main merge lands and main's publish workflows complete, bump the minor in `version.json` on develop (e.g. `3.16` → `3.17`) via an isolated `bump-version-X.Y` PR. Without it, develop's next prerelease version numbers fall below main's just-shipped stable.
- Don't recommend `git push --force` or `--force-with-lease`; both rulesets enforce `non_fast_forward`.
- `version.json`'s `publicReleaseRefSpec` is `^refs/heads/main$` — bumping the base `version` field is the only manual versioning action.

Expand Down Expand Up @@ -300,11 +302,13 @@ dotnet husky run

### GitHub Actions

- **publish-release.yml**: Multi-runtime matrix build (win, linux, osx x x64/arm/arm64)
- **publish-periodic-docker-release.yml**: Multi-arch Docker builds (linux/amd64, linux/arm64)
- **test-pull-request.yml**: PR validation
- Version info: `version.json` with Nerdbank.GitVersioning format
- Branches: `main` (stable releases), `develop` (pre-releases)
Two-phase model — reusable `*-task.yml` workflows orchestrated by two entry points:

- **test-pull-request.yml**: PR validation. `changes` (dorny/paths-filter) → always-on `unit-test` (Husky) + path-gated `smoke-build` (reduced, no-push) → `Check pull request workflow status` aggregator (ruleset-bound name; requires `changes` succeeded).
- **publish-release.yml**: the **sole publisher** (`push` + weekly `schedule` + `workflow_dispatch`). A `setup` job computes the branch list + publish gate; the `publish` matrix builds both branches via `build-release-task.yml` (executable 7-RID matrix + multi-arch Docker `linux/amd64,linux/arm64` + GitHub release), then `tool-versions`, `docker-readme` (main only), `date-badge` (main only).
- Reusable tasks: `build-release-task.yml`, `build-executable-task.yml`, `build-docker-task.yml`, `build-toolversions-task.yml`, `build-dockerreadme-task.yml`, `build-datebadge-task.yml`, `get-version-task.yml`. All thread a required `branch` input (config keys off it, never `github.ref_name`) plus `ref`/`smoke`.
- Version info: `version.json` with Nerdbank.GitVersioning format. `get-version-task.yml` surfaces `SemVer2`, the assembly versions, and `GitCommitId` (used to pin the release `target_commitish`).
- Branches: `main` (stable releases, `latest`), `develop` (pre-releases, `develop`).

### Docker

Expand Down Expand Up @@ -462,9 +466,131 @@ Check states with `HasFlag()`, combine with `|=`

## Git and Commit Rules

**These rules are absolute — no exceptions:**

- **Never make git commits.** All commits must be cryptographically signed (SSH/GPG). AI coding agents cannot produce signed commits. Stage changes with `git add` and leave `git commit` to the developer, who must run it in their own environment where signing keys are available.
- **Default to staging, not committing.** Stage changes with `git add` and leave `git commit` to the developer unless explicitly authorized to commit for the current ask ("commit this", "open a PR"). Authorization is scope-bound to that task.
- **All commits must be cryptographically signed (SSH/GPG)** — branch protection rejects unsigned commits. Signing depends on environment config (`commit.gpgsign`, a `user.signingkey`, a loaded agent). If signing isn't configured, **do not commit** — stop at `git add` and surface it. Verify first: `git config --get commit.gpgsign && ssh-add -L`.
- **Never force push.** Do not run `git push --force` or `git push --force-with-lease`. Force pushing rewrites shared branch history and is blocked by branch protection rules.
- **Never run destructive git commands** (`git reset --hard`, `git checkout .`, `git restore .`, `git clean -f`) without explicit developer instruction.
- **Staging is the limit.** Prepare changes and stage files; the developer handles all commits and pushes.
- **The `develop → main` release merge is maintainer-only.** Drive `feature → develop` PRs end-to-end when authorized (commit, push, Copilot review loop, squash-merge), but never self-merge a release to `main`.

## GitHub Copilot Review Runbook

Provider-specific mechanics for driving GitHub Copilot reviews entirely via `gh`/GraphQL — no manual UI clicks. The review-loop *contract* (re-request on every push, verify head-SHA coverage, triage, reply + resolve, escalate when stuck) is in [AGENTS.md → Merging a PR](../AGENTS.md#merging-a-pr); this section is how to make Copilot reliably execute it.

### Triggering and Polling

Auto-review on push is configured (the branch ruleset's `copilot_code_review` rule with `review_on_push: true`) but fires inconsistently — treat it as best-effort. After every push, **re-request a review programmatically** via the GraphQL `requestReviews` mutation, passing the Copilot reviewer's bot node id in `botIds`. This drives the loop end-to-end without a maintainer clicking "re-request review" in the UI.

> **The reviewer login differs by API — this is intentional, not a typo.** In **GraphQL** (`gh api graphql` and `gh pr view --json reviews`, which is GraphQL-backed) the `Bot.login` is `copilot-pull-request-reviewer` — **no `[bot]` suffix**. In the **REST** API (`gh api repos/.../issues|pulls/...`) the same account's `user.login` is `copilot-pull-request-reviewer[bot]` — **with** the suffix. Each query below uses the correct form for its API; match the API, not a single spelling, when adapting them. (The prose elsewhere referring to `copilot-pull-request-reviewer[bot]` is describing the REST/display login.)

```sh
# 1. PR node id + the Copilot reviewer's bot node id (read from any existing
# Copilot review; the reviewer login is `copilot-pull-request-reviewer`).
PR_NODE=$(gh pr view <N> --json id --jq '.id')
BOT_ID=$(gh api graphql -f query='
{
repository(owner: "ptr727", name: "PlexCleaner") {
pullRequest(number: <N>) {
reviews(first: 50) { nodes { author { __typename login ... on Bot { id } } } }
}
}
}' --jq '[.data.repository.pullRequest.reviews.nodes[]
| select(.author.login == "copilot-pull-request-reviewer")
| .author.id] | first')

# 2. Re-request a Copilot review on the current head.
gh api graphql -f query='
mutation($pr: ID!, $bot: ID!) {
requestReviews(input: { pullRequestId: $pr, botIds: [$bot], union: true }) {
pullRequest { id }
}
}' -F pr="$PR_NODE" -F bot="$BOT_ID"
```

The bot node id is read from an existing Copilot review, so step 1 needs at least one prior review on the PR — auto-review-on-open normally supplies the first. If none exists yet and auto-review didn't fire, request `Copilot` once through the GitHub PR UI to seed it, then use the mutation for every subsequent re-request. The Copilot reviewer bot's global node id is `BOT_kgDOCnlnWA` (login `copilot-pull-request-reviewer`) if you need to skip discovery.

**Do NOT post `@Copilot review` as a PR comment.** That triggers the Copilot *coding agent* (`copilot-swe-agent[bot]`), which makes code changes rather than posting a review.

Known non-working request paths (use the `requestReviews` mutation instead):

- `POST /requested_reviewers` with `reviewers=[Copilot]` can return 200 but no-op.
- `copilot-pull-request-reviewer` as a requested reviewer slug returns 422.

### Verify Review Covered Current Head

Before merging, confirm Copilot reviewed the current PR head SHA. Copilot may respond as a formal review (carries an exact commit SHA) or an issue comment (no SHA). Check both.

```sh
PR_HEAD=$(gh pr view <N> --json headRefOid --jq '.headRefOid')

# 1. Formal review — exact SHA match.
gh pr view <N> --json reviews --jq \
'.reviews[] | select(.author.login=="copilot-pull-request-reviewer") | .commit.oid' \
| grep -q "$PR_HEAD" && echo "covered via formal review"

# 2. Issue comment — show the most recent Copilot comment for manual
# confirmation. This is the REST API, so the login carries the `[bot]` suffix.
gh api repos/ptr727/PlexCleaner/issues/<N>/comments --jq \
'[.[] | select(.user.login=="copilot-pull-request-reviewer[bot]")] | last | {created_at, body: .body[:200]}'
```

Coverage is confirmed when (1) exits 0. For issue comments (path 2), body content is the only reliable signal — `created_at` is not (commit timestamps can predate the push). Treat path (2) as confirmed only when the comment body explicitly refers to the current changes.

### Bounded Retry Workflow

If a review did not run on the current head:

1. Wait briefly and check head-SHA coverage (above).
1. Re-request via the `requestReviews` mutation; fall back to the GitHub PR UI only if the mutation no-ops.
1. Retry up to two more times (three total).
1. If still missing, mark the review blocked and escalate to the maintainer with what was attempted.

### Reply and Thread Resolution Workflow

List unresolved threads (`first: 100` + cursor pagination; if `hasNextPage`, re-run with `after: "<endCursor>"`):

```sh
gh api graphql -f query='
{
repository(owner: "ptr727", name: "PlexCleaner") {
pullRequest(number: <N>) {
reviewThreads(first: 100) {
nodes {
id isResolved path
comments(first: 1) { nodes { author { login } body } }
}
pageInfo { hasNextPage endCursor }
}
}
}
}' | jq '
.data.repository.pullRequest.reviewThreads |
(.pageInfo | "hasNextPage=\(.hasNextPage) endCursor=\(.endCursor)"),
(.nodes[] | select(.isResolved == false))
'
```

Reply on a thread, then resolve it:

```sh
gh api graphql -f query='
mutation($threadId: ID!, $body: String!) {
addPullRequestReviewThreadReply(input: { pullRequestReviewThreadId: $threadId, body: $body }) {
comment { id }
}
}' -F threadId="PRRT_..." -F body="Fixed in <SHA>: <one-line summary>."

gh api graphql -f query='
mutation($threadId: ID!) {
resolveReviewThread(input: { threadId: $threadId }) { thread { id isResolved } }
}' -F threadId="PRRT_..."
```

Issue-level Copilot comments (those in `issues/<N>/comments`) have no resolution action — reply if the finding warrants it; no resolution step is possible.

Reply-body conventions:

- Accepted bug/style fix: include fixing commit SHA and a one-line summary.
- Declined style comment: cite the rule (AGENTS.md or CODESTYLE) and the existing-tree precedent.
- Declined architecture proposal: one-sentence rationale.

A PR is mergeable when `mergeStateStatus == CLEAN` and there are 0 unresolved threads on the current head. After the final push, sweep-resolve stale older threads for removed code paths.
10 changes: 9 additions & 1 deletion .github/workflows/build-datebadge-task.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@ name: Build BYOB date badge task

on:
workflow_call:
inputs:
# Logical branch this badge run is for. The badge only updates on
# `main`; the publisher passes the branch explicitly so a scheduled run
# building `develop` doesn't try to write the main badge. Required (no
# `github.ref_name` fallback) so the gate can't silently misfire.
branch:
required: true
type: string

jobs:

Expand All @@ -16,7 +24,7 @@ jobs:
run: echo "date=$(date)" >> $GITHUB_OUTPUT

- name: Build BYOB date badge step
if: ${{ github.ref_name == 'main' }}
if: ${{ inputs.branch == 'main' }}
uses: RubbaBoy/BYOB@24f464284c1fd32028524b59607d417a2e36fee7 # v1.3.0
with:
name: lastbuild
Expand Down
86 changes: 65 additions & 21 deletions .github/workflows/build-docker-task.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,63 +8,107 @@ on:
required: false
type: boolean
default: false
# Git ref to check out / version (empty = default checkout ref).
ref:
required: false
type: string
default: ''
# Logical branch driving config and tags (`main` => Release/`latest`,
# anything else => Debug/`develop`). Required (no `github.ref_name`
# fallback): the publisher builds develop from a run whose
# `github.ref_name` is `main`, so a silent fallback would mistag it.
# The orchestrator always passes it explicitly.
branch:
required: true
type: string
# Smoke mode: build `linux/amd64` only (no QEMU/arm64), never push, and
# skip the shared registry `cache-to` so PR builds don't pollute the
# release buildcache. Used for fast PR feedback.
smoke:
required: false
type: boolean
default: false

jobs:

get-version:
name: Get version information job
uses: ./.github/workflows/get-version-task.yml
secrets: inherit
with:
ref: ${{ inputs.ref }}

build-docker:
name: Build Docker image job
runs-on: ubuntu-latest
needs: [get-version]
strategy:
matrix:
include:
- file: ./Docker/Dockerfile
platforms: linux/amd64,linux/arm64
tags: |
docker.io/ptr727/plexcleaner:${{ github.ref_name == 'main' && 'latest' || 'develop' }}
docker.io/ptr727/plexcleaner:${{ needs.get-version.outputs.SemVer2 }}

steps:

- name: Checkout step
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
ref: ${{ inputs.ref }}

# QEMU only exists to emulate arm64. Smoke builds are amd64-only, so
# skip it entirely to save the emulation setup cost.
- name: Setup QEMU step
uses: docker/setup-qemu-action@06116385d9baf250c9f4dcb4858b16962ea869c3 # v4.1.0
if: ${{ !inputs.smoke }}
uses: docker/setup-qemu-action@06116385d9baf250c9f4dcb4858b16962ea869c3 # v4.1.0
with:
platforms: linux/amd64,linux/arm64

- name: Setup Buildx step
uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0
uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0
with:
platforms: linux/amd64,linux/arm64
platforms: ${{ inputs.smoke && 'linux/amd64' || 'linux/amd64,linux/arm64' }}

# Always login to Docker Hub, not just on push, to benefit from
# higher rate limits with a Docker subscription for pulls and cache
# Always login to Docker Hub, not just on push, to benefit from higher
# rate limits with a Docker subscription for pulls and cache reads on
# every build (including smoke). This is a CONSCIOUS choice over gating
# login on `inputs.push`: the trade-off is that fork PRs without access
# to the Docker Hub secrets cannot run the Docker smoke build. Acceptable
# because PlexCleaner's contribution flow is same-repo branch PRs +
# Dependabot (App token); a fork-PR Docker smoke path would alternatively
# gate this step on `inputs.push` and accept anonymous-rate-limited reads.
- name: Login to Docker Hub step
uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4.2.0
uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4.2.0
with:
username: ${{ secrets.DOCKER_HUB_USERNAME }}
password: ${{ secrets.DOCKER_HUB_ACCESS_TOKEN }}

- name: Docker build and push step
uses: docker/build-push-action@f9f3042f7e2789586610d6e8b85c8f03e5195baf # v7.2.0
uses: docker/build-push-action@f9f3042f7e2789586610d6e8b85c8f03e5195baf # v7.2.0
with:
context: .
push: ${{ inputs.push }}
file: ${{ matrix.file }}
tags: ${{ matrix.tags }}
platforms: ${{ matrix.platforms }}
cache-from: type=registry,ref=docker.io/ptr727/plexcleaner:buildcache
cache-to: type=registry,ref=docker.io/ptr727/plexcleaner:buildcache,mode=max
file: ./Docker/Dockerfile
tags: |
docker.io/ptr727/plexcleaner:${{ inputs.branch == 'main' && 'latest' || 'develop' }}
docker.io/ptr727/plexcleaner:${{ needs.get-version.outputs.SemVer2 }}
platforms: ${{ inputs.smoke && 'linux/amd64' || 'linux/amd64,linux/arm64' }}
# Branch-scoped registry cache. READ both branches' caches — the
# layers are nearly identical (only BUILD_CONFIGURATION differs), so
# a main build can seed from develop's cache and vice versa — but
# WRITE only this branch's own tag, and only when actually pushing.
# Gating the export on `inputs.push` (not just `!smoke`) means a
# non-publishing build never writes the shared registry cache or
# needs Docker Hub write creds — smoke builds (always push=false) are
# covered too. Branch-scoping is what lets the publisher's weekly
# matrix build main and develop concurrently in one run without the
# two legs overwriting a single shared cache (destroying hit rates).
# `mode=max` caches the multi-stage builder layers (the expensive
# `dotnet publish`); `ignore-error=true` keeps a transient cache
# export failure from failing an otherwise-good publish. Registry
# (not gha) cache because the weekly publish cadence would let gha's
# 7-day unused-entry eviction drop branch caches between publishes.
cache-from: |
type=registry,ref=docker.io/ptr727/plexcleaner:buildcache-main
type=registry,ref=docker.io/ptr727/plexcleaner:buildcache-develop
cache-to: ${{ inputs.push && format('type=registry,ref=docker.io/ptr727/plexcleaner:buildcache-{0},mode=max,ignore-error=true', inputs.branch) || '' }}
build-args: |
LABEL_VERSION=${{ needs.get-version.outputs.SemVer2 }}
BUILD_CONFIGURATION=${{ github.ref_name == 'main' && 'Release' || 'Debug' }}
BUILD_CONFIGURATION=${{ inputs.branch == 'main' && 'Release' || 'Debug' }}
BUILD_VERSION=${{ needs.get-version.outputs.AssemblyVersion }}
BUILD_FILE_VERSION=${{ needs.get-version.outputs.AssemblyFileVersion }}
BUILD_ASSEMBLY_VERSION=${{ needs.get-version.outputs.AssemblyVersion }}
Expand Down
Loading