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
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ scripts/repo_gate.py text eol=lf
scripts/pr_review.py text eol=lf
scripts/test_prose_lint.py text eol=lf
scripts/test_repo_gate.py text eol=lf
scripts/test_pr_review.py text eol=lf

# uv regenerates uv.lock with LF on every platform; pin it so git enforces LF on checkout/renormalize and a
# CRLF-default repo does not fight the tool on every `uv lock`/`uv sync`. A repo with no lockfile is unaffected.
Expand Down
11 changes: 9 additions & 2 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,17 @@ Auto-review on push is configured (via the branch ruleset's `copilot_code_review

**A review with no inline comments is still a completed review - not a failure, and not a reason to ask the maintainer to re-trigger.** Copilot very often posts a single formal review (GraphQL `state: COMMENTED`) whose body ends with "...reviewed N of N changed files ... and generated no comments" and adds **zero** inline threads. That review carries the head `commit.oid` and fully satisfies the loop - it is the clean-pass success case. Never read "no inline comments" as "the review didn't run," and never re-request or escalate to the maintainer because comments are absent.

**Read the low-confidence findings, which are not inline threads.** A review body can carry a collapsed `<details>` block headed "Comments suppressed due to low confidence", and those findings appear nowhere in `reviewThreads`, so a loop that polls threads alone never sees them and reports a clean pass. They have been right repeatedly, including a rule stated more broadly than its check enforced and a check that skipped fenced blocks in every rule but one. Read the body of every review, investigate each suppressed finding on the same footing as an inline one, and answer it in the PR conversation, since a suppressed finding has no thread to reply on or resolve.
**Read the low-confidence findings, which are not inline threads.** A review body can carry a collapsed `<details>` block of findings Copilot withheld from the inline threads, and those findings appear nowhere in `reviewThreads`, so a loop that polls threads alone never sees them and reports a clean pass. **Match the block on more than one phrasing.** Its heading has appeared both as `Suppressed comments (N)` and as "Comments suppressed due to low confidence", so a filter keyed on either one alone silently reports zero suppressed findings on a review that has them - the same false clean this rule exists to prevent, one level up in the detection. They have been right repeatedly, including a rule stated more broadly than its check enforced and a check that skipped fenced blocks in every rule but one. Read the body of every review, investigate each suppressed finding on the same footing as an inline one, and answer it in the PR conversation, since a suppressed finding has no thread to reply on or resolve.

```sh
gh api repos/<owner>/<repo>/pulls/<N>/reviews --jq '.[] | select(.body | contains("low confidence")) | .body'
# `test` with an alternation, not `contains` on one phrasing: the heading wording has changed.
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.
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"
```

**Round 1 is normally auto-seeded - 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
6 changes: 5 additions & 1 deletion .github/workflows/validate-task.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ jobs:
- name: Checkout code step
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0

# Doc linters run as pinned action wrappers. editorconfig-checker's action is install-only, so it runs via Docker.
# Doc linters run as pinned action wrappers.
# The editorconfig-checker action is install-only, so it runs via Docker instead.
- name: Lint Markdown step
uses: DavidAnson/markdownlint-cli2-action@8de2aa07cae85fd17c0b35642db70cf5495f1d25 # v24.0.0
with:
Expand Down Expand Up @@ -53,11 +54,14 @@ jobs:

# Each gate in scripts/ is proven by a case that reintroduces the fault it catches.
# Standard library only, so no install step and no dependency to pin.
# The audit engine self-test is offline, so it runs here rather than only on an owner sweep.
- name: Run script self-tests step
run: |
set -Eeuo pipefail
python3 scripts/test_prose_lint.py
python3 scripts/test_repo_gate.py
python3 scripts/test_pr_review.py
python3 spec/audit.py --selftest

- name: Check repo gates step
run: python3 scripts/repo_gate.py
Expand Down
2 changes: 2 additions & 0 deletions AUDIT.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ Write `reports/<repo>/audit.md` from [`reports/_template.md`][template]: a dimen

**Findings are a point-in-time snapshot - stamp them and re-verify before acting.** [`spec/audit.py`][audit-runner] prints a run stamp (`audit run <UTC> | hub <sha>`) and, per repo, the exact commit it read (`@ <branch>@<sha>`). Anything derived from a run - a report, and especially an **onboarding or conformance issue** - quotes that stamp, so a reader can tell whether it still applies. A convergence issue is generated from the audit, never composed by hand: `spec/audit.py --issue <repo>` emits a ready-to-file title and body from that repo's live findings (grouped into must-fix, converge, and could-not-verify), so the issue content cannot drift from what the audit actually found and regenerates as the repo changes.

**Verify a convergence before it is promoted with `--branch`.** `spec/audit.py --branch <ref> <repo>` reads that ref instead of the repo's registry `groundTruthBranch`, so a repo can audit its own `develop` while the work is still in flight rather than discovering the gaps after `main` has moved. The registry is not edited, the run is still read-only, and the run stamp names the override so a finding cannot be mistaken for one against ground truth. A ref that does not resolve is a single error naming it, never a baseline's worth of file-absent letters.

**Re-running the audit needs a full hub clone with git history.** The verbatim stale-vs-modified classification walks the canonical's history (`git log` / `git show` from the hub root), so a shallow clone or a files-only checkout cannot answer "matches a past hub revision" and those findings are unreliable there. A downstream agent verifying one finding without the full history can instead compare against the current hub canonical on `main` - the whole file for a file-level unit, or the named `## heading` block for a verbatim section - which decides current-match but not stale-vs-modified. An agent picking up such an issue **re-runs the audit first and acts on the live result, not the pasted findings**: a repo moves between filing and pickup, so a stale block leads an agent to "fix" what is already fixed (re-requesting secrets that exist, attempting a no-op forward-sync). State the findings as evidence for *why* the issue was filed, never as the current state.

**Reconcile `driftNotes` in the same pass.** A registry `driftNote` records a *current* deviation from the baseline. Once the deviation is resolved the note is deleted, not left describing finished work - hand-maintained prose drifts silently otherwise. `spec/audit.py` flags this: when a repo audits clean but a note still asserts outstanding work ("pending", "not yet", "missing", "behind", ...), it raises a drift finding naming the note.
Expand Down
Loading
Loading