Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
2 changes: 1 addition & 1 deletion .agents/skills/agent-conduct/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Read `GOVERNANCE.md` "Verification Discipline" before reporting success on anyth
- **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/<owner>/<repo>/contents/<path>?ref=<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.
- **PR-bound work runs `local-strict-review` before the claim, and records the pass.** 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, and recording that pass with `scripts/local_review.py` so a capture point can check it. Every push toward a pull request owes one, the fix pushes answering review findings included, which is the round it is most often skipped on.
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Resolved

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`.

Expand Down
12 changes: 8 additions & 4 deletions .agents/skills/drive-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent.
## 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.
2. Commit the work, run `local-strict-review` and record its pass in the order that skill gives,
then push the branch and open the feature -> develop PR if it does not exist yet. A push the
`pre-push` hook refuses is that gate working rather than an obstacle to route around, and that
skill's refusal table says what each refusal means and what clears it.
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
Expand Down Expand Up @@ -102,8 +104,10 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent.

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).
- Real, so fix it, then step 2's own order again before replying with the fixing commit SHA
(outcome 1). This is the round the pass is most often skipped on, since the fix looks small and
the branch was already reviewed once, and a fix push carries content no pass has read exactly as
the first push did.
- 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).
Expand Down
95 changes: 91 additions & 4 deletions .agents/skills/local-strict-review/SKILL.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ publisher passes `branch: ${{ github.ref_name }}`, which the tasks forward and r
`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.
the target list is expressed. A repo still curates, in `test-pull-request.yml`, its
path-filter entry, that filter's output, and the `smoke-build` enable-forward, all three together
per D6.4, since an entry nothing consumes never smoke-builds the target. And, for a package
target, the `publish-nuget` or `publish-pypi` job in its own `publish-release.yml`, since
`id-token: write` belongs at that one entry point. CodeGen, versioning, merge-bot, and Dependabot
are target-agnostic.

## Orchestration vs. build: the override seam

Expand Down Expand Up @@ -62,22 +64,30 @@ and project-path inputs its targets need.
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
hook or a build-nuget hook per output, each uploading `release-asset-<branch>-<target>`. This is where the
.NET `dotnet publish` or `dotnet build` lives, though a package push does not. 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
- **Package-registry pushes** (NuGet.org, PyPI): both are split, and the push never sits in the
hook. OIDC trusted publishing validates the token's `job_workflow_ref` claim, which names the
workflow the job actually ran from, so a push from a hub-hosted task is rejected at the token
exchange, NuGet.org answering `HTTP 401` and PyPI under its own code. And because D7.2 has a callee
declare `permissions:` only where every caller grants that scope at startup, the release task's
jobs declare none and run under the calling job's whole grant, so a push anywhere inside that
task would put `id-token: write` on every job in it. The
build-nuget hook uploads a `nuget-build-<branch>` artifact for a separate `publish-nuget` job in
the caller's own `publish-release.yml`, which authenticates through `NuGet/login` and therefore
carries `id-token: write` (plus `actions: write` to delete the artifact it consumed), *and* also
uploads a `release-asset-*` (.7z) for the GitHub release. PyPI is the same shape: 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-*`.
`publish-release.yml` does the OIDC Trusted-Publishing upload, behind an `environment: pypi`
gate and with `skip-existing: true` (`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
`build-release-task.yml`, pushes the default branch multi-arch (amd64+arm64) and any other
branch `amd64`-only, 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`
Expand Down
8 changes: 5 additions & 3 deletions .agents/skills/pr-review-conduct/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ 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.
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. Follow that skill's own ordering and record each pass, which is what a capture point such as this repository's `pre-push` hook reads. A push that hook refuses is the gate working rather than an obstacle to route around, and that skill's refusal table says what each refusal means and what clears it.

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.
Expand All @@ -105,8 +105,10 @@ After an authorized merge, run the `repo-worktree` post-merge cleanup procedure

## 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
1. **Real, so fix it.** Take the fix through `local-strict-review` the same way step 1's push
went, then reply with the fixing commit SHA. A branch already reviewed once
has not been reviewed for the fix, which is the round this gets dropped on and the churn
`local-strict-review` exists to stop. 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.
Expand Down
8 changes: 4 additions & 4 deletions .agents/skills/workflow-ci-contract/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,16 @@ description: >-
- **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).
- **Two layers.** Orchestration (the PR entry workflow, publisher, version and release jobs) is generic and standard at the job level. Build leaves (the `build-<target>` tasks) are repo-owned. Inputs like `github`/`dockerhub`/`expect_release_assets` live on the orchestrator, a leaf receives `ref`/`branch`/`smoke` and whatever else its target needs, a derived `push` among them where that leaf pushes, so assert each input in the layer that declares it. A package target declares no push input on either layer, its push living in a separate `publish-<target>` job in the repo's own publisher. 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` and `build-docker` `needs:` entries, the `changes` paths-filter entry and output, the `smoke-build` enable-forward, and a package target's `publish-<target>` job (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.
- **Concurrency**: top-level workflows use `group: '${{ github.workflow }}-${{ github.ref }}'` with `cancel-in-progress: true`. Two are documented exceptions. The publisher takes a global ref-independent group with `cancel-in-progress: false`, so publishes serialize and never cancel mid-push. The merge-bot takes `cancel-in-progress: false` and keys on the PR number rather than `github.ref`, per D8.1, so each PR queues independently and every event runs to completion.
- **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.
- **Permissions validate before `if:`**, so a callee declares `permissions:` only where every caller grants that scope at startup and otherwise declares none, running under the calling job's grant. A callee's extra scope (`actions: write` for cleanup) 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.
Expand All @@ -38,7 +38,7 @@ description: >-
- **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.
- **A build failure blocks every publish target**: `github-release` needs every build, the terminal registry pusher guards with `!failure() && !cancelled()`, and a package target's separate `publish-<target>` job `needs:` the release-task call, 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`.

Expand Down
Loading
Loading