From 768c3822e98778da9887f7948335bd0998ce8d5c Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 3 Aug 2026 15:39:08 -0700 Subject: [PATCH 1/2] Re-vendor the eight verbatim carries the hub advanced past The hub's first audit of this repo (ProjectTemplate#547) found eight verbatim units behind the canonical at hub main `3a7cc64`. Six are stale copies and two never arrived at all, and the distinction matters, because a stale copy still states the rule in an older form while an absent one states nothing. ## Absent, so this is the first time the rule lands here - `AGENTS.md > Fleet Bootstrap` says where the canonical rules live and routes by the state this repository is actually in. Without it, an agent starting here has nothing telling it the hub exists. - `GOVERNANCE.md > Representative Data in Agent-Authored Text` is the rule that agent-authored text illustrates with constructed data rather than with data observed in the maintainer's environment. ## Stale, so the rule was here in an older form - `AGENTS.md > Context and Delegation Discipline` gains the rule that a wait separates three outcomes and says which one it reached. - `AGENTS.md > Where the Rules Live` gains the row routing to the representative-data rule above. - `GOVERNANCE.md > Git and Commit Rules` gains "Commit means commit and push". - `GOVERNANCE.md > Verification Discipline` gains "A launched process is not a result, and a cause nobody observed is not a diagnosis". - `GOVERNANCE.md > PR Review Etiquette` gains the whole "Every Finding Ends in an Action" subsection, so the review loop arrives with the five outcomes that close a finding rather than without them. - `repo-config/configure.sh` takes the payload-driven check mode. The copy here compared the merge methods and the required-status-check contexts by name; the canonical compares every parameterized rule's whole parameters object in both directions, so a rule added live that no payload declares is now caught. TODO.md says not to hand-edit `configure.sh` or the ruleset payloads. Re-vendoring from the hub canonical is the sanctioned exception and the reason that rule exists. The two ruleset payloads are deliberately not touched: ProjectTemplate#545 changed them an hour ago on the hub's `develop`, and ground truth is the hub's `main`, so that one converges when it promotes. The intro line is adjusted from two things to four, because the file now holds the bootstrap as well. ## Verification - Every verbatim unit byte-matches the canonical after LF normalization: the three `AGENTS.md` sections, the eighteen `GOVERNANCE.md` sections, `configure.sh`, and `.markdownlint-cli2.jsonc`. The only two that differ are `Devcontainer` and `Repository Layout`, both intent fidelity. - `repo-config/configure.sh check ptr727/Blog release` exits 0 under the new stricter check mode, so the live rulesets and settings still conform, which is the check that would have caught a bad re-vendor. - markdownlint, editorconfig-checker, and the JSON and prose gates are clean, and all 20 rows of the rules map resolve to a carried section except the one the table itself marks hub-only. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 19 ++++++++++-- GOVERNANCE.md | 35 +++++++++++++++++++-- repo-config/configure.sh | 67 ++++++++++++++++++++++++++++------------ 3 files changed, 96 insertions(+), 25 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 96974b3..67935ee 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,7 +2,7 @@ **Blog** is the source and deployment pipeline for a personal technical blog, a Hugo static site served by Caddy behind a reverse proxy. It holds the content, the media, the URL contract the site must honor, and the release tooling that builds and publishes it. -This file is the entry point every coding agent reads first, and it holds only two things: the rules for managing context and delegation, which apply to every task, and a map of where every other rule lives. The rule text itself is in [`GOVERNANCE.md`](./GOVERNANCE.md), one section per topic. Code style lives in [`CODESTYLE.md`](./CODESTYLE.md), the CI/CD workflow contract in [`WORKFLOW.md`](./WORKFLOW.md), and the deploy, rollback, and server procedures in [`OPERATIONS.md`](./OPERATIONS.md). +This file is the entry point every coding agent reads first, and it holds only four things: this project's own conventions, the bootstrap that says where the canonical rules live and which procedure to follow for the state this repository is actually in, the rules for managing context and delegation, which apply to every task, and a map of where every other rule lives. The rule text itself is in [`GOVERNANCE.md`](./GOVERNANCE.md), one section per topic. Code style lives in [`CODESTYLE.md`](./CODESTYLE.md), the CI/CD workflow contract in [`WORKFLOW.md`](./WORKFLOW.md), and the deploy, rollback, and server procedures in [`OPERATIONS.md`](./OPERATIONS.md). Treat this file and `GOVERNANCE.md` as authoritative for cross-cutting rules, and do not restate their rules elsewhere. This project's own conventions and behavioral contracts live here, **not** in [`.github/copilot-instructions.md`](./.github/copilot-instructions.md), because that file targets GitHub Copilot / VS Code specifically, while this file and `GOVERNANCE.md` are the agent-agnostic ones every coding agent is directed to read, so any rule a reviewer must honor has to live in one of those two files to be provider-independent. @@ -15,6 +15,19 @@ This site has served the same domain across earlier platforms, so its whole risk - **`content/` is an archive.** It carries sixteen years of text. Prose, spelling, and style sweeps do not reach it, and rewriting it corrupts provenance rather than improving style. - **A gate proves itself by failing.** Every check here is demonstrated against a deliberate break before it is trusted, because a gate that has only ever passed is indistinguishable from one that checks nothing. +## Fleet Bootstrap + +This repository is governed by a shared template, and the canonical rules, machine-readable spec, and procedures live in `github.com/ptr727/ProjectTemplate`. Fetch that repository before acting on anything about conformance, carried content, repository settings, or standing a repository up, because a carried copy here can be stale or absent and the hub is the only authority on what this repository is supposed to hold. This section is byte-locked across every repository in the fleet, so it reads identically wherever it is found, and it is the entry point whenever nothing else present says where the rules are. + +Route by what this repository currently holds rather than by what it is expected to hold, since the two differ exactly when this section matters most. + +- **No repository yet, or a local tree with no remote.** Follow the hub's `STANDUP.md` from section 0. That file is hub-only and deliberately not carried, because a repository needing it cannot be relied on to hold a current copy. Note that nothing in it creates the GitHub repository, which is an outward-facing write requiring explicit permission, so section 0A is the list handed to the maintainer before anything else starts. +- **A repository with no carried instruction set, or a partial one.** Carry the baseline per the hub's `STANDUP.md` sections 1A and 2, which resolve what this repository is owed from its declared types and workflow model. Absent files are not drift to re-vendor, they are a baseline that never arrived, and the two are fixed differently. +- **A repository with the instruction set, current or stale.** Follow the hub's `AUDIT.md` end to end, then apply what it finds per its section 10. An audit that reports drift and stops is half the procedure. +- **A repository that believes it is conformant.** Run the audit anyway and commit the report, because conformance asserted without a report is conformance nobody can check. This is the same procedure as the case above and is listed separately only because it is the one most often skipped. + +Two rules bound every path above. **Read the hub's `main` branch as ground truth**, since that is the promoted and gated state, and read `develop` only to detect divergence. And **the audit is read-only**: it produces a report and never edits the repository it measures, so a fix is a separate, reviewable change. + ## Context and Delegation Discipline An agent session is billed on the context it carries, not the work it does. Every request re-reads the whole accumulated context, so a token added early is paid for again on every request that follows, and a long session bills its last task for every earlier one. These are cost rules. None of them licenses doing less work, skipping verification, or shipping something unreviewed. @@ -22,7 +35,7 @@ An agent session is billed on the context it carries, not the work it does. Ever ### Session Scope - **One deliverable, one session.** A session covers one branch and one deliverable, and ends when that work merges. A multi-step task is one deliverable and stays in one session. Two unrelated tasks are two sessions even when they run back to back. -- **End a session at any of these, without being asked:** the branch changes, the pull request merges, the next task is unrelated to the last, or a third review round opens on the same pull request. +- **End a session at any of these, without being asked:** the branch changes, the pull request merges, or the next task is unrelated to the last. A review round is none of them. A loop still producing findings is the deliverable in progress, and a round count is not a reason to leave one open. - **Hand off in a file, never in context.** Close a session by writing at most 2 KB to a scratch file: branch, pull request link, what is done, the next command. A summary held in context is re-billed until the session ends, and a summary on disk is read once by whoever needs it. - **Re-derive state, do not carry it.** "This session already has the context" is the signal to split, not to continue. Context that has gone stale is worse than absent, because a file read hundreds of requests ago no longer describes the file. - **Compaction is a fallback, not the strategy.** It restarts context from a floor and climbs again, where a fresh session starts from zero. @@ -54,6 +67,7 @@ If a rule you were given does not cover what you find, stop and report it. Do no ``` - **Wait in a background process, not in a poll loop.** A review or CI wait is a sequence of near-identical requests, each billed for whatever context it happens to carry. Run the wait as one backgrounded command that returns when the condition is met. +- **A wait separates three outcomes, and says which one it reached.** The condition was met, it has not been met yet, and the wait cannot reach it at all are three different results, and a backgrounded wait that emits nothing renders all three identically. Run the command once in the foreground and read its output before backgrounding it, because a wait is only as good as the command inside it, and an unsupported flag on the installed tool version exits non-zero with an empty stdout that every naive test reads as "nothing yet". Never let a fallback stand in for a failed command, since `|| echo '[]'`, `|| true`, and `2>/dev/null` convert an error into that same reading, which is the suppression the write-safety rules already forbid on a mutation. Make the wait emit on failure as loudly as on success, so silence means "still running" and nothing else, and bound it, so a condition that is never coming ends in a report rather than in another wait. ## Where the Rules Live @@ -64,6 +78,7 @@ Every rule below is a level-two section of [`GOVERNANCE.md`](./GOVERNANCE.md). R | Why the rules are shaped this way | `Foundational Principles` | | Recording a durable lesson or updating governance | `Durable Knowledge and Self-Improvement` | | Any push, API mutation, comment, label, or merge | `Repository Boundaries and Write Safety` | +| Quoting data into a comment, commit, test, or doc | `Representative Data in Agent-Authored Text` | | Committing, signing, rebasing, force-pushing | `Git and Commit Rules` | | Branch choice, promotion, keeping branches in sync | `Branching Model` | | Releasing, version bumps, publishing | `Release Model` | diff --git a/GOVERNANCE.md b/GOVERNANCE.md index c071a23..e498f13 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -26,9 +26,18 @@ A state-changing GitHub call is the highest-blast-radius thing an agent does her - **Never fabricate, guess, or reuse an identifier passed to a write.** Every id a state-changing call consumes (a node id, a numeric id, a thread or comment id) is captured from a live query in the **same** session into a variable and passed from there. Do not hand-type an id, guess it, recall it from memory or an earlier session, or copy it from documentation or an example. Ids commonly resolve **globally**, so a wrong-but-valid id does not fail. It writes to the wrong target, in someone else's repository. If a query returns no id, stop rather than invent one to proceed. - **A write is never a probe, and a write's output is never suppressed.** Never fire a state-changing call to see whether it works: decide it should happen, make it happen, and read the result. Never append output-discarding redirection or a force-success tail to a mutation (for example `>/dev/null`, `2>/dev/null`, `&>/dev/null`, `|| true`, `|| :`, `|| echo`), because the write's output is exactly what must be read. A write that appears to fail is **verified, not assumed harmless**, because the operation may have succeeded on the server while the client reported an error, so confirm the actual state before retrying or moving on. The ban targets hiding a *failure*. An ad-hoc call's response is the only signal you get, so `>/dev/null 2>&1`, `|| true`, and `|| echo`, which swallow the error stream or force success, are never acceptable on one. A committed script under `set -e` is a narrow exception: it may send a write's *stdout* to `/dev/null` to drop the success-response noise, because stderr stays visible and a failed write still aborts loudly (`repo-config/configure.sh` does exactly this). The exception is stdout-only suppression inside a reviewed, fail-loud script, never `2>&1` or a force-success tail, and never an ad-hoc command. +## Representative Data in Agent-Authored Text + +Agent-authored text illustrates with data the agent constructed, never with data it observed in the maintainer's environment. This binds every surface an agent writes: pull request and issue comments, review replies, commit messages, code, tests, fixtures, and docs. Reading real data is unrestricted, and what is bounded is what an agent copies out of the environment into text that is committed or posted. The rule holds for a private repository as much as a public one, since a repository's audience changes with one settings toggle while the text stays exactly where it was written, and it holds where the data is the maintainer's own, since the exposure happens on their behalf before they can weigh it. + +- **Synthetic evidence is the better evidence, not a weaker substitute.** A case constructed to carry the defect demonstrates it exactly and any reader can re-run it, where observed data proves the same thing and can never be reproduced by anyone else. A filename built to contain a newline is a complete proof of a newline-handling defect, and the real directory it was found in adds nothing the proof needed. Reaching for observed data to make a finding more convincing inverts which of the two is the stronger evidence. Where observed data is what revealed the defect, name its shape, meaning the property that triggers the fault, and construct a case that carries that property. +- **The exposure is one-way.** A public comment is fetched, cached, and indexed the moment it posts, so editing it afterwards is mitigation rather than a fix, and the edit leaves the original readable in the comment's edit history to anyone who can read the repository. Text that has already landed is reported to the maintainer rather than quietly rewritten, since the decision on what to do about it, deletion included, is theirs. Do not quote the exposed data again while reporting or investigating it, because a transcript, an issue, or a commit message written about the exposure reproduces it somewhere new. +- **No checker closes this.** A pattern finds an absolute home path or a drive letter, and gating that subset is worth doing as a floor. The exposure this rule exists for was name-shaped, and a name is not pattern-detectable, so a search of the offending text for path-shaped strings returns nothing while the names sit in plain sight. A gate here catches the easy half, and mistaking it for the answer is what stops anyone looking at the other half, which is why this is a judgment an agent applies rather than a check it waits for. + ## 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. @@ -212,6 +221,7 @@ The checks that separate work actually done from work that merely reports succes - **Never edit source through a shell heredoc when the text carries backslash escapes.** The shell consumes the escape and writes an invisible control character in its place, so a `\b` inside a regex becomes a backspace and the pattern silently matches nothing while every test still passes. Use a file-editing tool for such text. When a check inspects text for control characters, use `str.isprintable()` rather than a codepoint floor, since DEL and the Unicode format characters sit above 32 and are equally invisible in a diff. - **Never edit an active `.code-workspace` file.** A workspace file rewritten on disk can make VS Code reload the window, and a reload destroys the running agent session's context, so the work in flight is lost with nothing to catch it, and the trigger is not fully characterized (an agent's edit has caused the reload where a human's identical edit did not). Surface the needed change for the maintainer to apply by hand. - **A green check is not evidence the work happened.** A skipped job and a passing job are indistinguishable in the aggregated required check. When a job exists to exercise something, confirm from its log that it ran and produced the output it promises. +- **A launched process is not a result, and a cause nobody observed is not a diagnosis.** "The watcher is armed" names a process rather than a finding, so what gets reported is the output that process produced, and where it produced none, that absence is the report. The failure it prevents is an agent standing still on a condition that was met half an hour earlier, having announced the wait and never read it. Naming an external cause for such a stall afterwards, a throttle or a quota that appears nowhere in the record, turns a local defect into a story about someone else and closes the investigation on the wrong party, so read the record for the cause before naming one, and where the record does not carry it, report the cause as unknown. - **A workflow change is only fully exercised by CI.** Extracting a `run:` block and executing it locally validates the script and nothing else, because `secrets: inherit`, `permissions:`, `needs:` wiring, and reusable-workflow inputs resolve only in a real run. - **A review flags an instance, so fix the class.** When a reviewer cites one stale claim, one silent-narrowing pattern, or one mis-worded contract, sweep for its siblings before replying. Reviewers sample rather than enumerate. @@ -248,28 +258,47 @@ Drive the loop to green, meaning a review confirmed on the latest head SHA and e For provider-specific mechanics (how to request review, query review state, post replies, resolve threads), see the **GitHub Copilot Review Runbook** in [.github/copilot-instructions.md](./.github/copilot-instructions.md). This file owns the contract, and that file owns the mechanics. +### Every Finding Ends in an Action + +**A finding is closed by one of five outcomes, and a round count is never one of them.** The loop runs until no finding stands, however many rounds that takes, because the number of rounds measures how much was found rather than whether the work is done. A finding parked, waited out, or superseded by a push is still open. + +1. **It is real, so fix it.** Reply with the fixing commit SHA. +2. **It is not real, so disprove it in the thread**, with the command and its output, the code path that makes it impossible, or the rule that governs it. The proof is addressed to the reviewer as much as to the maintainer, since a decline it can read is what stops it raising the same thing next round. An assertion is not a proof and does not close a finding. +3. **It is real and deliberately not being fixed, which is the maintainer's call and not the agent's.** Say what the finding is, why the fix is unwanted, and get an explicit answer. Never suppress one by silence, by resolving the thread, or by an answer that reads as a decline while conceding the point. +4. **It is real and worth doing later, so file the issue first and reply with its link.** A deferral recorded only in a thread is lost the moment the pull request merges, so the issue is what carries it and the link is what proves it exists rather than being intended. This is for work the change did not create: an adjacent defect the reviewer noticed in passing, or a fix too large to ride along. It does not cover a defect in the change under review, because filing an issue about a bug you are about to merge is outcome 3 in other clothes, and that one is the maintainer's to decide. +5. **It keeps coming back, so fix the class rather than the instance.** A finding raised repeatedly against correct code is a defect in what the code communicates, not in the reviewer. Give it what it lacks: the non-obvious *why* as a comment where the code cannot state it, a clearer name, a narrower interface, or the rule change where the rule is what is wrong. A comment written for this earns its place under the comment rules like any other, so it states the why, stays short, and never cites a rule or addresses the reviewer. Making the noise stop is worth doing well, because a reviewer that repeats itself trains the reader to skim it, and skimming is how a real finding gets missed. + ### 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. - **Style/convention** - the comment cites a rule from this file or a language-specific style guide. Two cases: - The cited rule matches what the existing codebase already does -> fix the offending code. - - The cited rule contradicts what's in the tree, or industry norm -> **update the rule instead of the code**. The rule is wrong, not the code. Bouncing the same code across rounds is the symptom of a wrong rule. Heuristic: three rounds on the same style category means the rule needs adjusting and the user should authorize the rule change. + - The cited rule contradicts what's in the tree, or industry norm -> **update the rule instead of the code**. The rule is wrong, not the code. Bouncing the same code across rounds is the symptom of a wrong rule, so treat the recurrence itself as the finding and take it to the user for the rule change (outcome 5 above), rather than counting rounds until some threshold licenses it. - **Architectural opinion** - the comment proposes a different design ("constrain this to disabled-by-default", "move it elsewhere", "add a runtime guardrail"). This is judgment, not a bug. Surface it to the user with a recommendation, and don't apply it unilaterally. ### 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//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//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 `, `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 ` 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: - **Genuine design trade-off** surfaces (fail-open vs fail-closed, narrow vs broad refactor scope, "should we add a guardrail or trust the docstring"). Triage, recommend, ask. -- **Repeated friction** across rounds without convergence, which is the rule-needs-updating signal. Stop, summarize the pattern, and let the user authorize the rule change. +- **A recurring finding** the code keeps attracting, which is the fix-the-class signal. Summarize the pattern and bring the remedy, whether that is the rule change or what the code has to say differently to stop earning it. +- **A finding you judge real but do not want fixed**, which is outcome 3 above and is never the agent's call to make quietly. - **Architectural redesign** is requested rather than a bug fix. Surface with a recommendation, and never apply it unilaterally. Anti-pattern: don't keep flipping the code on the same style point. Flip the rule once and stick to the rule. diff --git a/repo-config/configure.sh b/repo-config/configure.sh index 86a88d4..41a3a53 100755 --- a/repo-config/configure.sh +++ b/repo-config/configure.sh @@ -17,9 +17,15 @@ # The develop ruleset is develop.json where the model is PR-gated, or operational/develop.json for direct signed pushes. # Applying the same configuration twice changes nothing, so the mode is idempotent. # -# The check mode is the read-only inverse, where every applied ruleset, setting, and security feature must match. -# The ruleset and settings assertions are driven by the committed payloads, so they stay repo-agnostic. -# That also survives the GitHub API normalizing a stored ruleset, comparing rule presence, merge methods and required checks rather than a byte diff. +# The check mode is the read-only inverse, and it verifies the same three groups apply writes. +# The ruleset and static-settings assertions are driven by the committed payloads, so they stay repo-agnostic. +# A ruleset is checked on enforcement, on the rule-type set compared in both directions, and on the whole parameters object of every parameterized rule. +# Comparing the parameters object rather than named fields means a parameter added to a payload is audited with no change here. +# Both directions matter, since a rule added live that the payload never declared is drift this catches. +# That still survives the GitHub API normalizing a stored ruleset, since the comparison is over parsed JSON with sorted keys rather than a byte diff. +# The derived settings apply computes are asserted by name rather than from a payload, meaning has_discussions and default_branch. +# The two Dependabot security features are asserted the same way, since apply enables them and no payload declares them. +# What is unaudited is a static setting absent from settings.json, since only that group is payload-driven. # Secrets are per-repo (see spec/secrets.json) and not checkable from a standalone carry, so they are a manual-verify note. set -Eeuo pipefail @@ -185,7 +191,7 @@ jq_has() { jq -e "$@" >/dev/null 2>&1; } gh_ok() { gh api "$@" >/dev/null 2>&1; } check_ruleset() { # payload-file - the live ruleset must match the committed policy, driven by the payload - local file="$1" rname id live t want got wantc gotc want_enf + local file="$1" rname id live t want got want_enf if [ ! -e "$file" ]; then fail "ruleset payload $file missing"; return; fi rname="$(jq -r '.name // empty' "$file")" if [ -z "$rname" ]; then fail "ruleset payload $file has no name"; return; fi @@ -194,23 +200,35 @@ check_ruleset() { # payload-file - the live ruleset must match the committed pol if ! live="$(gh api "repos/$repo/rulesets/$id")"; then fail "ruleset '$rname' - could not read live state"; return; fi want_enf="$(jq -r '.enforcement' "$file")" assert "ruleset '$rname' enforcement = $want_enf" test "$(jq -r '.enforcement' <<<"$live")" = "$want_enf" - # Every rule type the committed payload declares must be present live (payload-driven, so repo-agnostic). + # The live rule-type set must equal the payload's, compared in both directions. + # Checking only that each payload type is present live misses a rule someone added by hand. + # That is drift this script exists to catch, and it passed as clean before. + local want_types got_types + if ! want_types="$(jq -r '[.rules[].type] | sort | join(",")' "$file")"; then + fail "ruleset payload $file did not parse"; return + fi + if [ -z "$want_types" ]; then fail "ruleset payload $file declares no rules"; return; fi + got_types="$(jq -r '[.rules[].type] | sort | join(",")' <<<"$live")" + assert "'$rname' rule set = $want_types" test "$got_types" = "$want_types" + # Every parameterized rule is compared on its whole parameters object rather than on selected fields. + # Naming fields one at a time meant a payload could declare a parameter the check never read. + # Review-thread resolution, stale-review dismissal, and the status-check policy flags all went unverified that way. + # Comparing the object keeps the check payload-driven: a parameter added to a payload is audited with no code change. + # Keys are sorted on both sides, so key order from the API cannot read as drift. + # Set-like arrays are sorted too, since the API guarantees no order and the previous per-field comparison sorted them explicitly. + # Dropping that would turn array order into false drift. + # A scalar array sorts directly, and required_status_checks sorts by context, its identifying field. + local ptypes norm + norm='def n: walk(if type=="array" then (if length==0 then . elif (all(.[]; type=="string" or type=="number")) then sort elif (all(.[]; type=="object" and has("context"))) then sort_by(.context) else . end) else . end); n' + ptypes="$(jq -r '[.rules[] | select(has("parameters")) | .type] | .[]' "$file")" while IFS= read -r t; do + [ -z "$t" ] && continue # shellcheck disable=SC2016 # $t is a jq --arg variable, not a shell expansion - assert "'$rname' enforces rule '$t'" jq_has --arg t "$t" '.rules[] | select(.type==$t)' <<<"$live" - done < <(jq -r '.rules[].type' "$file") - # For pull_request, the live merge methods must match the payload, which is the develop=squash and main=merge policy. - if jq_has '.rules[] | select(.type=="pull_request")' "$file"; then - want="$(jq -c '[.rules[]|select(.type=="pull_request").parameters.allowed_merge_methods[]]|sort' "$file")" - got="$(jq -c '[.rules[]|select(.type=="pull_request").parameters.allowed_merge_methods[]]|sort' <<<"$live")" - assert "'$rname' merge methods = $want" test "$got" = "$want" - fi - # For required_status_checks, the live required contexts must match the payload. - if jq_has '.rules[] | select(.type=="required_status_checks")' "$file"; then - wantc="$(jq -c '[.rules[]|select(.type=="required_status_checks").parameters.required_status_checks[].context]|sort' "$file")" - gotc="$(jq -c '[.rules[]|select(.type=="required_status_checks").parameters.required_status_checks[].context]|sort' <<<"$live")" - assert "'$rname' required checks = $wantc" test "$gotc" = "$wantc" - fi + want="$(jq -S -c --arg t "$t" "[.rules[] | select(.type==\$t) | .parameters] | first | $norm" "$file")" + # shellcheck disable=SC2016 + got="$(jq -S -c --arg t "$t" "[.rules[] | select(.type==\$t) | .parameters] | first | $norm" <<<"$live")" + assert "'$rname' rule '$t' parameters match the payload" test "$got" = "$want" + done <<<"$ptypes" } check_settings() { @@ -219,11 +237,20 @@ check_settings() { if ! live="$(gh api "repos/$repo")"; then fail "could not read repository settings"; return; fi # Static settings are driven from settings.json, so the check never drifts from the file. # Add a key there and it is audited here automatically. + # The payload is parsed into a variable before the loop rather than streamed from a process substitution. + # A jq failure inside `done < <(...)` leaves the loop body unexecuted without tripping set -e. + # Every static setting would then report as checked and passing while nothing was compared, a false clean. + local pairs + if ! pairs="$(jq -r 'to_entries[] | "\(.key)\t\(.value)"' "$settings_file")"; then + fail "settings payload $settings_file did not parse"; return + fi + # A payload that parses to nothing is a floor failure rather than a clean run, so it is asserted. + if [ -z "$pairs" ]; then fail "settings payload $settings_file declares no keys"; return; fi while IFS=$'\t' read -r key want; do # shellcheck disable=SC2016 # $k is a jq --arg variable, not a shell expansion got="$(jq -r --arg k "$key" '.[$k]' <<<"$live")" assert "setting $key = $want" test "$got" = "$want" - done < <(jq -r 'to_entries[] | "\(.key)\t\(.value)"' "$settings_file") + done <<<"$pairs" # Dynamic settings apply sets: has_discussions (public repos only), default_branch (main, if it exists). private="$(jq -r '.private' <<<"$live")" wantdisc=true; [ "$private" = "true" ] && wantdisc=false From 92566886af5ccd7a1ca3e9bd158c7f905dc84174 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 3 Aug 2026 15:46:59 -0700 Subject: [PATCH 2/2] State the hub ref the carry now matches The report named `6501479`, the ref the baseline was carried from. The re-vendor in this branch moves every verbatim unit to `3a7cc64`, so the line named a ref the tree no longer matched the moment the rest of this change landed. Co-Authored-By: Claude Opus 5 (1M context) --- reports/Blog/audit.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reports/Blog/audit.md b/reports/Blog/audit.md index fb655b2..38846ab 100644 --- a/reports/Blog/audit.md +++ b/reports/Blog/audit.md @@ -3,7 +3,7 @@ Self-audit of this repository against its own committed ground truth, per [AUDIT.md](../../AUDIT.md). Read-only, and confined to this repository. **Date:** 2026-08-01 -**Hub ref carried:** `ptr727/ProjectTemplate` `main` `6501479` +**Hub ref carried:** `ptr727/ProjectTemplate` `main` `3a7cc64` **Declared:** `types: ["source-only"]`, `workflowModel: release`, `lineEndings: "lf"` ## Verdict