diff --git a/.github/dependabot.yml b/.github/dependabot.yml index d796b022..7b5319ff 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,9 +1,9 @@ # https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file # -# github-actions is the only ecosystem this repo ships. Dual-target (main + develop) so both branches -# stay current independently of the develop -> main release cadence; the merge-bot auto-merges each -# base with its per-base method. See catalog/snippets/configs/dependabot.yml for the multi-ecosystem -# reference (nuget, uv) a code-shipping repo uses. +# The github-actions ecosystem is the only one this repo ships. +# Both main and develop are targeted so each stays current independently of the develop to main release cadence. +# The merge-bot auto-merges each base with its per-base method. +# See catalog/snippets/configs/dependabot.yml for the multi-ecosystem reference, covering nuget and uv, that a code-shipping repo uses. version: 2 updates: diff --git a/.github/workflows/merge-bot-pull-request.yml b/.github/workflows/merge-bot-pull-request.yml index 596e436c..a579cd47 100644 --- a/.github/workflows/merge-bot-pull-request.yml +++ b/.github/workflows/merge-bot-pull-request.yml @@ -3,15 +3,14 @@ name: Merge bot pull request action # Auto-merges in-repo bot PRs (Dependabot, codegen): enable on opened/reopened, disable on a maintainer push. # - Merge method by base: develop = squash, main = merge. # - App token, not GITHUB_TOKEN: fires downstream workflows on merge, and grants write on read-only Dependabot PRs. -# - pull_request_target, not pull_request: jobs hold the App key, so the workflow + action SHAs resolve from the -# trusted base, not PR head. Safe because no job checks out PR code (each runs gh pr merge by URL). +# - pull_request_target rather than pull_request, since jobs hold the App key, so the workflow and action SHAs resolve from the trusted base rather than the PR head. +# This is safe because no job checks out PR code, each one running gh pr merge by URL. on: pull_request_target: types: [opened, reopened, synchronize] -# Concurrency keys on the PR number, not github.ref (the base branch under pull_request_target, which would -# serialize every bot PR against it), so each PR queues independently. cancel-in-progress: false so a follow-up -# synchronize doesn't cancel an in-flight opened run before it enables auto-merge. +# Concurrency keys on the PR number rather than on github.ref, which under pull_request_target is the base branch and would serialize every bot PR against it, so each PR queues independently. +# The cancel-in-progress setting is false so a follow-up synchronize does not cancel an in-flight opened run before it enables auto-merge. concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number }} cancel-in-progress: false @@ -21,7 +20,8 @@ jobs: merge-dependabot: name: Merge dependabot pull request job runs-on: ubuntu-latest - # Dependabot PRs from this repo (not forks). Only on opened/reopened so the disable job stays sticky. + # Dependabot PRs from this repo rather than from forks. + # Only on opened or reopened, so the disable job stays sticky. if: >- (github.event.action == 'opened' || github.event.action == 'reopened') && github.event.pull_request.user.login == 'dependabot[bot]' && @@ -59,8 +59,9 @@ jobs: merge-codegen: name: Merge codegen pull request job runs-on: ubuntu-latest - # Codegen PRs from this repo. Head/base pairing is enforced strictly (codegen-main->main, codegen-develop-> - # develop). Only on opened/reopened so the disable job stays sticky. + # Codegen PRs from this repo. + # Head and base pairing is enforced strictly, codegen-main to main and codegen-develop to develop. + # Only on opened or reopened, so the disable job stays sticky. if: >- (github.event.action == 'opened' || github.event.action == 'reopened') && github.event.pull_request.user.login == 'ptr727-codegen[bot]' && @@ -101,8 +102,9 @@ jobs: merge-upstream-version: name: Merge upstream version pull request job runs-on: ubuntu-latest - # Upstream-version bump PRs from the App. Head/base pairing is enforced (upstream-version-main->main, - # upstream-version-develop->develop). Only on opened/reopened so the disable job stays sticky. + # Upstream-version bump PRs from the App. + # Head and base pairing is enforced, upstream-version-main to main and upstream-version-develop to develop. + # Only on opened or reopened, so the disable job stays sticky. if: >- (github.event.action == 'opened' || github.event.action == 'reopened') && github.event.pull_request.user.login == 'ptr727-codegen[bot]' && @@ -143,8 +145,9 @@ jobs: disable-auto-merge-on-maintainer-push: name: Disable auto-merge on maintainer push job runs-on: ubuntu-latest - # Fires when a maintainer pushes to a bot's branch (synchronize, actor != bot). Disables auto-merge so the - # maintainer's commits don't merge with the bot's, and they re-enable it manually. The disable call is idempotent. + # Fires when a maintainer pushes to a bot's branch, meaning a synchronize whose actor is not the bot. + # It disables auto-merge so the maintainer's commits do not merge with the bot's, and the maintainer re-enables it manually. + # The disable call is idempotent. if: >- github.event.action == 'synchronize' && github.event.pull_request.head.repo.full_name == github.repository && diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index 264960a4..ec149e86 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -3,8 +3,8 @@ name: Publish project release action on: workflow_dispatch: -# A publish is a deliberate dispatch, so runs serialize on one group and queue rather than cancel, so a run is -# never left with a half-created GitHub release. +# A publish is a deliberate dispatch, so runs serialize on one group and queue rather than cancel. +# That leaves no run with a half-created GitHub release. concurrency: group: ${{ github.workflow }} cancel-in-progress: false @@ -18,8 +18,9 @@ jobs: permissions: contents: read - # Publish the dispatched branch (main => release, develop => prerelease): NBGV computes the tag from the ref, then - # a GitHub release is created (tag + auto source archive + README + LICENSE). Source-only repo - no build targets. + # Publish the dispatched branch, where main gives a release and develop a prerelease. + # NBGV computes the tag from the ref, then a GitHub release is created carrying the tag, the auto source archive, README and LICENSE. + # This repo is source-only, so it has no build targets. publish: name: Publish project release job runs-on: ubuntu-latest @@ -56,8 +57,8 @@ jobs: # Create-or-refresh: every trigger here is a dispatch, so an existing tag is refreshed, never skipped # (the exists-gate belongs to the multi-trigger reusable form, where a scheduled re-run must no-op). - # target_commitish pins the tag to the exact built commit (GitCommitId), not the default branch. The release is - # the tag plus GitHub's auto source archive, README, and LICENSE - no build assets (source-only). + # The target_commitish input pins the tag to the exact built commit, GitCommitId, rather than to the default branch. + # The release is the tag plus GitHub's auto source archive, README and LICENSE, carrying no build assets because the repo is source-only. - name: Create GitHub release step uses: softprops/action-gh-release@718ea10b132b3b2eba29c1007bb80653f286566b # v3.0.1 with: diff --git a/.github/workflows/test-pull-request.yml b/.github/workflows/test-pull-request.yml index 115f52c1..a8cd0bb4 100644 --- a/.github/workflows/test-pull-request.yml +++ b/.github/workflows/test-pull-request.yml @@ -18,8 +18,8 @@ jobs: permissions: contents: read - # GitHub Actions does not support required status checks on conditional jobs, so a single always-run aggregator gates - # the merge. Its name is the ruleset-bound required status-check context - rename it and the ruleset context together. + # GitHub Actions does not support required status checks on conditional jobs, so a single always-run aggregator gates the merge. + # Its name is the ruleset-bound required status-check context, so rename it and the ruleset context together. check-workflow-status: name: Check pull request workflow status job runs-on: ubuntu-latest diff --git a/.github/workflows/validate-task.yml b/.github/workflows/validate-task.yml index 816ad566..8a0ce712 100644 --- a/.github/workflows/validate-task.yml +++ b/.github/workflows/validate-task.yml @@ -68,13 +68,13 @@ jobs: - name: Check repo gates step run: python3 scripts/repo_gate.py - # The charset, duplicate-word and spelling rules are clean tree-wide, so they gate. + # The charset, duplicate-word, spelling and comment rules are clean tree-wide, so they gate. # Every other prose rule reports in the step below without gating. - name: Check prose step - run: python3 scripts/prose_lint.py . --check charset --check dupword --check spelling + run: python3 scripts/prose_lint.py . --check charset --check dupword --check spelling --check comment-wrap --check comment-case # Warn-only, and visible rather than absent: an unrun check is one nobody acts on. - # The backlog is corrected as each file is next edited, never swept. + # The backlog is corrected as each file is next edited, or cleared in a deliberate batch. - name: Report prose backlog step continue-on-error: true - run: python3 scripts/prose_lint.py . --check charset-unknown --check semicolon --check dash --check comment-wrap --check comment-case --summary + run: python3 scripts/prose_lint.py . --check charset-unknown --check semicolon --check dash --summary diff --git a/OPERATIONS.md b/OPERATIONS.md index b09383f4..e5bc83c9 100644 --- a/OPERATIONS.md +++ b/OPERATIONS.md @@ -8,7 +8,7 @@ What verifying a change here requires, including the part CI cannot perform. The ### Run the gates the way CI runs them -CI passes explicit `--check` lists, and a bare `python3 scripts/prose_lint.py [file]` runs `DEFAULT_RULES`, which is those two lists together. What differs is the exit code rather than the coverage: CI gates on `charset`, `dupword` and `spelling` and reports the other five warn-only, where a bare run exits non-zero on any of the eight. `sentence-split` is in neither and is asked for by name. Run the CI invocations: +CI passes explicit `--check` lists, and a bare `python3 scripts/prose_lint.py [file]` runs `DEFAULT_RULES`, which is those two lists plus `home-path`. What differs is the exit code rather than the coverage: CI gates on `charset`, `dupword`, `spelling`, `comment-wrap` and `comment-case` and reports the other three warn-only, where a bare run exits non-zero on any of the nine. `sentence-split` is in neither and is asked for by name. Run the CI invocations: ```sh python3 scripts/test_prose_lint.py @@ -17,18 +17,18 @@ python3 scripts/test_pr_review.py python3 spec/audit.py --selftest python3 host-setup/agent-safety/gh-write-guard.py --selftest python3 scripts/repo_gate.py -python3 scripts/prose_lint.py . --check charset --check dupword --check spelling -python3 scripts/prose_lint.py . --check charset-unknown --check semicolon --check dash --check comment-wrap --check comment-case --summary +python3 scripts/prose_lint.py . --check charset --check dupword --check spelling --check comment-wrap --check comment-case +python3 scripts/prose_lint.py . --check charset-unknown --check semicolon --check dash --summary for f in registry/*.json spec/*.json repo-config/*.json; do jq empty "$f"; done python3 spec/validate.py docker run --rm --pull=always -v "$PWD":/check --workdir /check mstruebing/editorconfig-checker:latest ``` -Two gaps in that list are CI's rather than this runbook's, reproduced here so a local run matches CI rather than quietly exceeding it. The `jq` glob covers `repo-config/*.json` and does not reach `repo-config/operational/develop.json`, so a malformed operational payload passes. And `sentence-split` is implemented and tested but named by no invocation, so nothing runs it. +Three gaps in that list are CI's rather than this runbook's, reproduced here so a local run matches CI rather than quietly exceeding it. The `jq` glob covers `repo-config/*.json` and does not reach `repo-config/operational/develop.json`, so a malformed operational payload passes. And `sentence-split` is implemented and tested but named by no invocation, so nothing runs it. The third is `home-path`, which is in `DEFAULT_RULES` and so runs on every bare local run, yet is named by neither CI list, so the pattern-detectable half of the representative-data rule gates nothing in CI. It is clean tree-wide today, which is why the gap is a hole rather than a backlog. Run the `editorconfig-checker` line before pushing any new file. This repository defaults to CRLF, most tooling writes LF, and a new file therefore fails that check on its first CI run rather than locally. -The first prose invocation gates. The second reports the backlog that is corrected as each file is next edited, and it exits non-zero locally whenever findings exist. It is warn-only in CI because the workflow step sets `continue-on-error: true`, not because the command is lenient, so a non-zero exit locally is the expected result rather than a problem. +The first prose invocation gates. The second reports the backlog that is corrected as each file is next edited, or cleared in a deliberate batch, and it exits non-zero locally whenever findings exist. It is warn-only in CI because the workflow step sets `continue-on-error: true`, not because the command is lenient, so a non-zero exit locally is the expected result rather than a problem. Scope a run to what changed, which matches the correct-as-next-edited rule: diff --git a/TODO.md b/TODO.md index 156a63e2..561d4263 100644 --- a/TODO.md +++ b/TODO.md @@ -31,12 +31,16 @@ One pull request clearing the prose findings the hub's own docs and spec still c - **Clear the remaining [#519][issue-519] prose backlog, outside the snippets.** The whole-tree figure moves as readily with a fix to the gate as with a fix to the prose, so it is re-measured rather than quoted. - **Blocked by** - Nothing. - **Issue** - [#519][issue-519], whose headline numbers are stale and whose four planned changes are two-thirds landed. - - **Checked** - `develop` at `a6d7a4b` on 2026-08-07, where `python3 scripts/prose_lint.py --summary` reports 373 violations across 26 files, and `catalog/snippets` reports 0. The snippets sweep took the tree from 557 across 45, of which 184 across 19 were snippets. + - **Checked** - `develop` at `c64e3e0` on 2026-08-07, where `python3 scripts/prose_lint.py --summary` reported 373 violations across 26 files and `catalog/snippets` reported 0. The 131 across 15 the comment batch leaves is a figure on the branch carrying that batch rather than on this anchor, and it becomes the anchor's own number when that branch merges. The snippets sweep took the tree from 557 across 45, of which 184 across 19 were snippets. - **Open** - Whether the gate becomes a carried file rather than a hub-only one, which "Reducing the Carried Surface Further" asks from the other direction. - **Settled** - `comment-wrap` and `comment-case` are in `DEFAULT_RULES` and `reports/` is exempt as a generated tree, which is why the figures differ from the 668 and 119 the issue records. - **Settled** - `sentence-split` is defined but excluded from `DEFAULT_RULES`, so a sweep never reports it and a wrapped sentence in Markdown prose is not a finding. - - **Settled** - The three largest files are not snippets and are Python comments rather than prose, being [`spec/audit.py`][audit] at 99, [`gh-write-guard.py`][write-guard] at 52, and [`spec/validate.py`][validate] at 41 when measured before the sweep. - - **Settled** - A comment opening on a lowercase identifier is the bulk of what `comment-case` still reports, and the rule intends those restructured rather than exempted. The exemptions the snippets sweep added cover a commented-out key and a definition label, and nothing wider. + - **Settled** - The backlog splits by surface rather than by file, since `comment-wrap` and `comment-case` lived entirely in non-Markdown comments while `dash` and `semicolon` live entirely in Markdown prose. That split is what made the comment half one reviewable batch, and it is the batch boundary the remainder inherits. + - **Settled** - The comment batch cleared 241 findings across 11 files and moved both rules to the gating CI step, since a rule swept clean but left warn-only regresses on the next edit with nothing reporting it. The three largest files were [`spec/audit.py`][audit] at 99, [`gh-write-guard.py`][write-guard] at 52, and [`spec/validate.py`][validate] at 41, all Python comments rather than prose. + - **Settled** - An ellipsis read as a sentence terminator, so `RUN_ON` reported one schematic comment line as two sentences and the split it asked for would have broken the fragment the line exists to show. The guard is that a dot preceded by a dot never terminates, fixed and tested before any prose moved, and the verdict diff in both directions was that one finding and nothing else. + - **Settled** - A comment opening on a lowercase identifier was the bulk of what `comment-case` reported, and the rule intends those restructured rather than exempted, which is what the batch did rather than widening any exemption. + - **Settled** - The remainder is prose in Markdown, and six of the fifteen files are carried, being [`GOVERNANCE.md`][governance] at 14, [`WORKFLOW.md`][workflow] at 14, [`CODESTYLE.md`][codestyle] at 6, [`.github/copilot-instructions.md`][copilot-instructions] at 4, `repo-config/README.md` at 2 and [`HISTORY.md`][history] at 1. So the next batch splits again at that line, since the carried half rewrites byte-locked sections and owes a re-vendor while the hub-only half owes nothing. + - **Settled** - `home-path` is in `DEFAULT_RULES` and is named by neither CI list, so the pattern-detectable half of the representative-data rule runs on every bare local run and gates nothing in CI. It is clean tree-wide, so this is a hole rather than a backlog, and it is recorded in [`OPERATIONS.md`][operations] beside the two gaps already named there. ### Giving the Fleet's Own Pins Something to Resolve Against @@ -405,6 +409,7 @@ Regenerate [reports/divergences.md][divergences-report] before using it as the w - **Issue** - [#365][issue-365] and [#483][issue-483]. - **Rides with** - Nothing on the hub, since the write-guard newline fix has landed on `develop` and a machine keeps running the old hook until the installer is re-run there. - **Detail** - A ticked row means the host-wide rules text and not the hook, since only running the installer deploys both layers, and the proxmox host proved that distinction by carrying the documentary half alone for eight days on the machine where the incident originated. + - **Detail** - The prose comment batch rewrote comments in [`gh-write-guard.py`][write-guard] and both installer wrappers, so every installed copy is now behind the hub by that much. The divergence is comment-only and changes no decision the hook takes, which the self-test confirms, so it is a re-run of the installer at the next visit rather than a correctness problem. - **Detail** - Honor the issue's own rule when filling a cell, that an unverified install command is worse than a blank, because a blank prompts a question while a wrong command produces a broken host and a false sense that setup succeeded. - **Detail** - The superseded safety section from [#364][issue-364] still sits above the canonical block in this host's rules file, so the two overlap. Removing it is a judgment call on a per-machine file, which is why it is surfaced rather than applied. @@ -468,6 +473,7 @@ Each was checked against the tree and has nothing left to do anywhere. Closing i [divergences-report]: ./reports/divergences.md [files]: ./spec/files.json [governance]: ./GOVERNANCE.md +[history]: ./HISTORY.md [markdownlint]: ./.markdownlint-cli2.jsonc [matrix]: ./reports/conformance-matrix.md [merge-bot]: ./.github/workflows/merge-bot-pull-request.yml @@ -486,5 +492,6 @@ Each was checked against the tree and has nothing left to do anywhere. Closing i [standup]: ./STANDUP.md [type-model]: ./spec/type-model.md [validate]: ./spec/validate.py +[workflow]: ./WORKFLOW.md [workflows]: ./catalog/snippets/workflows/ [write-guard]: ./host-setup/agent-safety/gh-write-guard.py diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index ae13c282..93ac819a 100644 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -35,7 +35,8 @@ from urllib.parse import quote # --- What counts as a GitHub write ------------------------------------------------------------------- -# gh subcommands that mutate. `gh api` is handled separately (it needs field/method inspection). +# The gh subcommands that mutate. +# The `gh api` command is handled separately, since it needs field and method inspection. _GH_WRITE_SUB = re.compile( r"""\bgh\s+(?: pr\s+(?:create|comment|close|merge|edit|review|reopen|ready|lock|unlock) @@ -49,7 +50,7 @@ ) _GH_API = re.compile(r"\bgh\s+api\b") _EXPLICIT_WRITE_METHOD = re.compile(r"(?:--method|-X)\s+(?:POST|PUT|PATCH|DELETE)\b", re.I) -# gh api with a field flag defaults to POST even without -X, so it is a write. +# A gh api call with a field flag defaults to POST even without -X, so it is a write. _API_FIELD_FLAG = re.compile(r"(?:^|\s)(?:-f|-F|--field|--raw-field|--input)\b") _GRAPHQL = re.compile(r"\bgh\s+api\b.*\bgraphql\b", re.S) _MUTATION = re.compile(r"\bmutation\b") @@ -58,11 +59,10 @@ _GIT_PUSH = re.compile(r"\bgit\b.*?\bpush\b", re.S) # --- Bypass-of-branch-rule detectors (Rule 4) -------------------------------------------------------- -# A git operation is denied when it would only succeed by bypassing an active branch rule - the harm is -# that the maintainer's admin identity CAN bypass, so a plain-looking push silently lands on a protected -# branch. The judgment is made against the branch's *live* rules (self-configuring: a code-style develop -# carries `pull_request` and is denied, a config-style develop does not and is allowed), except for the -# explicit-bypass flags below, which are the bypass by definition and need no query. +# A git operation is denied when it would only succeed by bypassing an active branch rule. +# The harm is that the maintainer's admin identity can bypass, so a plain-looking push silently lands on a protected branch. +# The judgment is made against the branch's *live* rules, which makes it self-configuring: a code-style develop carries `pull_request` and is denied, where a config-style develop does not and is allowed. +# The exception is the explicit-bypass flags below, which are the bypass by definition and need no query. # # Branches that fail CLOSED when their rules cannot be read - protected-by-default across every config. _PROTECTED_DEFAULT_ORDER = ("main", "master", "develop") @@ -71,27 +71,25 @@ _GH_ADMIN_MERGE = re.compile(r"\bgh\s+pr\s+merge\b[^\n|&;]*(?:^|\s)--admin\b") # --- Risk-pattern detectors -------------------------------------------------------------------------- -# Output-discard / force-success tails. Bare `2>&1` is NOT here: it merges stderr into stdout, leaving -# the output visible, so it is not suppression (and denying it would break `... 2>&1 | tee log`). +# Output-discard and force-success tails. +# A bare `2>&1` is deliberately not here, since it merges stderr into stdout and leaves the output visible, so it is not suppression, and denying it would break `... 2>&1 | tee log`. _SUPPRESS = re.compile(r">\s*/dev/null|&>\s*/dev/null|2>\s*/dev/null|\|\|\s*(?:true\b|echo\b|:)") -# A quoted argument value ("..." or '...'). Stripped before the suppression scan so a --body/--title -# that merely mentions `|| true` or `>/dev/null` as text is not mistaken for a real command tail. Real -# suppression tails are unquoted shell operators, so stripping quotes never hides an actual footgun. The -# double-quoted form allows `\"` escapes so an embedded quote does not end the span early; shell single -# quotes take no escapes, so their form is literal. +# A quoted argument value, in either double or single quotes. +# It is stripped before the suppression scan so a --body or --title that merely mentions `|| true` or `>/dev/null` as text is not mistaken for a real command tail. +# Real suppression tails are unquoted shell operators, so stripping quotes never hides an actual footgun. +# The double-quoted form allows `\"` escapes so an embedded quote does not end the span early. +# Shell single quotes take no escapes, so their form is literal. _QUOTED_SPAN = re.compile(r'"(?:\\.|[^"\\])*"' r"|'[^']*'") -# A GitHub global node id literal: an UPPERCASE prefix (PR_, PRRT_, IC_, BOT_, ...) + a long base64url -# body, or a legacy MD... base64 id. The uppercase prefix plus a >=12-char body keeps it from matching -# an ordinary underscored word in a reply body (e.g. body="fixed_the_thing_now", lowercase prefix). +# A GitHub global node id literal, being an uppercase prefix such as PR_, PRRT_, IC_ or BOT_ followed by a long base64url body, or a legacy MD-prefixed base64 id. +# The uppercase prefix plus a body of at least 12 characters keeps it from matching an ordinary underscored word in a reply body, such as body="fixed_the_thing_now" with its lowercase prefix. _NODE_ID_LITERAL = re.compile(r'^(?:[A-Z]{1,5}_[A-Za-z0-9_\-]{12,}|MD[A-Za-z0-9]{12,})$') # -F/-f name=VALUE, capturing the value - handles "quoted" and bare _FIELD_ASSIGN = re.compile(r"""(?:-F|-f|--field|--raw-field)\s+[A-Za-z_][\w]*=(?P'[^']*'|"[^"]*"|\S+)""") -# Every spelling gh accepts for the target flag: `--repo x`, `--repo=x`, `-R x`, `-R=x`, and the attached -# short form `-Rx`. A form left out is not a near-miss, it is a silent bypass of the whole repository -# scope, so the separator is matched rather than assumed to be a space. The look-behind requires the flag -# to start a shell token (whitespace before it, or the string start), which is where a real flag always -# sits, so a value that opens a quoted span (`--title "-Rowner/repo"`) is not read as a target. -# A mention inside prose (`--title "use -Rowner/repo"`) IS still read as a target and still denies. +# Every spelling gh accepts for the target flag, being `--repo x`, `--repo=x`, `-R x`, `-R=x`, and the attached short form `-Rx`. +# A form left out is not a near-miss, it is a silent bypass of the whole repository scope, so the separator is matched rather than assumed to be a space. +# The look-behind requires the flag to start a shell token, meaning whitespace before it or the string start, which is where a real flag always sits. +# A value that opens a quoted span, as in `--title "-Rowner/repo"`, is therefore not read as a target. +# A mention inside prose, as in `--title "use -Rowner/repo"`, is still read as a target and still denies. # A space precedes it exactly as one precedes a real flag, so no look-behind can separate the two. # Telling a flag from text needs argv-position parsing, the way _push_targets does it for git push. _EXPLICIT_REPO = re.compile(r"(?['\"]?)(?P[^\s'\"]+)(?P=q)") @@ -148,8 +146,8 @@ def _granted_targets(environ=None): def _target_permitted(target, origin, granted): """True when a write to target is in scope for a checkout whose origin is origin.""" - # Same owner covers the origin itself and every sibling repository, which is the case the maintainer - # works in daily. A different owner is the incident shape and needs the grant. + # Same owner covers the origin itself and every sibling repository, which is the case the maintainer works in daily. + # A different owner is the incident shape and needs the grant. if target[0] == origin[0]: return True return target in granted or (target[0], "*") in granted @@ -163,7 +161,7 @@ def _live_branch_rules(owner, repo, branch): """ try: r = subprocess.run( - # quote the branch: a name with `/` (feature/x) would otherwise split the API path. + # Quote the branch, since a name carrying `/`, such as feature/x, would otherwise split the API path. ["gh", "api", f"repos/{owner}/{repo}/rules/branches/{quote(branch, safe='')}", "--jq", "[.[].type]"], capture_output=True, text=True, timeout=10, ) @@ -195,7 +193,7 @@ def _current_push_branch(cwd): # Flags that consume the following token as a value, so the value is not a positional (remote/refspec). _PUSH_VALUE_FLAGS = {"-o", "--push-option", "--repo", "--receive-pack", "--exec"} -# git global options (before the subcommand) that consume the following token as their value. +# The git global options, which sit before the subcommand, that consume the following token as their value. _GIT_GLOBAL_VALUE_OPTS = {"-C", "-c", "--git-dir", "--work-tree", "--namespace", "--exec-path", "--config-env"} @@ -327,7 +325,7 @@ def _push_targets(cmd, cwd=None, current_branch=None): else: positionals.append(t) i += 1 - # positionals are [remote, refspec...]; a lone positional is the remote (a bare push). + # The positionals are the remote followed by any refspecs, and a lone positional is the remote, meaning a bare push. refspecs = positionals[1:] if len(positionals) >= 2 else [] branches = [] for rs in refspecs: @@ -377,9 +375,9 @@ def _check_bypass_flags(cmd): "This uses `gh pr merge --admin`, which merges past required reviews and status checks using " "admin power - a bypass of the merge gate. Merge only when the gate is satisfied." + _handoff(cmd) ) - # --no-verify / commit -n skip the git hooks, so they only matter as an actual arg to a git commit or - # push (other tools use --no-verify for unrelated things; shlex keeps a quoted mention out of the argv). - # `-n` is --no-verify only for commit; `git push -n` is --dry-run. + # The --no-verify flag and `commit -n` skip the git hooks, so they only matter as an actual argument to a git commit or push. + # Other tools use --no-verify for unrelated things, and shlex keeps a quoted mention out of the argv. + # The `-n` form is --no-verify only for commit, since `git push -n` is --dry-run. commit_lists = _git_subcommand_arglists(cmd, "commit") push_lists = _push_arg_lists(cmd) commit_bypass = any(("--no-verify" in a) or ("-n" in a) for a in commit_lists) @@ -447,17 +445,16 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, stands in for the live branch-rules query, and environ stands in for the process environment the maintainer's grant is read from. """ - # Fold shell line-continuations so a multi-line Bash invocation (`gh pr merge 5 \ --admin`) - # parses as one command; only backslash-newline is joined, so a real newline between commands still - # separates them. + # Fold shell line-continuations so a multi-line Bash invocation, such as `gh pr merge 5 \ --admin`, parses as one command. + # Only backslash-newline is joined, so a real newline between commands still separates them. cmd = re.sub(r"\\\r?\n", " ", cmd) - # Rule 4: a git operation that would only succeed by bypassing an active branch rule. Checked before - # the gh-write gate below, since `git commit --no-verify` is a bypass yet not a GitHub write. + # Rule 4 covers a git operation that would only succeed by bypassing an active branch rule. + # It is checked before the gh-write gate below, since `git commit --no-verify` is a bypass yet not a GitHub write. dec, reason = _check_bypass_flags(cmd) if dec == "deny": return dec, reason - # `_push_targets` tokenizes with shlex and keys off a real `git push` argv adjacency, so a push named - # only inside a quoted argument yields no target - the raw substring is just a cheap pre-filter. + # The `_push_targets` helper tokenizes with shlex and keys off a real `git push` argv adjacency, so a push named only inside a quoted argument yields no target. + # The raw substring is just a cheap pre-filter. if _GIT_PUSH.search(cmd): dec, reason = _check_push_bypass(cmd, cwd, origin, current_branch, rules_lookup) if dec == "deny": @@ -497,8 +494,8 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, if origin is None: origin = _origin_owner_repo(cwd) targets = [] - # Every occurrence, not the first: a compound command carries one target per invocation, and reading - # only the first checks the harmless one while the write after `&&` goes unexamined. + # Every occurrence is read rather than the first, since a compound command carries one target per invocation. + # Reading only the first checks the harmless one while the write after `&&` goes unexamined. for mr in _EXPLICIT_REPO.finditer(cmd): val = mr.group("r") if "/" in val and "<" not in val: @@ -507,9 +504,8 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, for m in _API_REPO_PATH.finditer(cmd): if "<" not in m.group("owner"): targets.append((m.group("owner").lower(), m.group("repo").lower())) - # Only runs when origin resolves (a git checkout): with no project context there is nothing to - # compare an explicit target against, so this check is skipped and rules 1-2 still apply. A node-id - # target is invisible here regardless - that is what rule 2 guards. + # This only runs when origin resolves, meaning a git checkout, since with no project context there is nothing to compare an explicit target against, so the check is skipped and rules 1 and 2 still apply. + # A node-id target is invisible here regardless, which is what rule 2 guards. if origin: granted = _granted_targets(environ) for t in targets: @@ -556,8 +552,9 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, ("gh api graphql -f query='mutation{resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"TODO_fixit\"", "allow", "short all-caps token is not a node id"), ] -# Rule-3 (repository scope) cases. Each carries the environment the grant is read from, so the run never -# depends on the environment the self-test happens to inherit. Origin is ptr727/plexcleaner throughout. +# Rule-3 cases, covering repository scope. +# Each carries the environment the grant is read from, so the run never depends on the environment the self-test happens to inherit. +# Origin is ptr727/plexcleaner throughout. _SCOPE_CASES = [ # (command, environ, expected_decision, label) ("gh issue create --repo ptr727/PhotoCleaner --title x --body y", {}, "allow", "sibling repo under the same owner"), @@ -569,8 +566,8 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, ("gh issue comment 5 -R mankatcheung/job-finder --body hi", {_ALLOW_ENV: "esphome/*"}, "deny", "the incident: a grant for one owner does not reach another"), ("gh issue create --repo esphome/esphome --title x", {_ALLOW_ENV: "not-an-owner-repo"}, "deny", "a malformed grant grants nothing"), ("GH_WRITE_GUARD_ALLOW=esphome/esphome gh issue create --repo esphome/esphome --title x", {}, "deny", "an inline env prefix is part of the command, not the hook's environment"), - # Every spelling of the target flag. A form the extraction misses is a silent bypass of rule 3, not a - # near-miss, so each is asserted against a foreign owner that must deny. + # Every spelling of the target flag. + # A form the extraction misses is a silent bypass of rule 3 rather than a near-miss, so each is asserted against a foreign owner that must deny. ("gh issue create --repo=esphome/esphome --title x", {}, "deny", "--repo=value equals form"), ("gh issue create -R=esphome/esphome --title x", {}, "deny", "-R=value equals form"), ("gh issue create -Resphome/esphome --title x", {}, "deny", "-Rvalue attached short form"), @@ -579,9 +576,10 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, ("gh issue create --repo ptr727/PhotoCleaner --title \"-Resphome/esphome\"", {}, "allow", "a value opening a quoted span is not a flag"), ] -# Rule-4 (branch-rule bypass) cases. Each carries its own branch->rules map so the run is deterministic -# and offline - the real hook queries the live rules, here rules_lookup is injected. current_branch -# stands in for the git resolution of a bare push. `None` rules mean the query could not be read. +# Rule-4 cases, covering branch-rule bypass. +# Each carries its own branch-to-rules map so the run is deterministic and offline, where the real hook queries the live rules and here rules_lookup is injected. +# The current_branch value stands in for the git resolution of a bare push. +# A `None` rules value means the query could not be read. _CODE_RULES = {"deletion", "non_fast_forward", "required_linear_history", "required_signatures", "pull_request", "required_status_checks", "copilot_code_review"} # code-style develop / any main _CONFIG_RULES = {"deletion", "non_fast_forward", "required_signatures"} # config-style develop: no pull_request @@ -649,9 +647,8 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, def _selftest(): - # Deterministic offline run: pin origin to ptr727/PlexCleaner (the incident repo) so the cross-origin - # case resolves without touching a real checkout. The gh-write cases inject empty rules + a feature - # current-branch so no case reaches the live branch-rules query. + # A deterministic offline run, pinning origin to ptr727/PlexCleaner, the incident repo, so the cross-origin case resolves without touching a real checkout. + # The gh-write cases inject empty rules and a feature current-branch so no case reaches the live branch-rules query. origin = ("ptr727", "plexcleaner") ok = True for cmd, want, label in _CASES: diff --git a/host-setup/agent-safety/install.ps1 b/host-setup/agent-safety/install.ps1 index 2274fea6..09d1f2f8 100644 --- a/host-setup/agent-safety/install.ps1 +++ b/host-setup/agent-safety/install.ps1 @@ -1,20 +1,21 @@ # Thin wrapper: run the cross-platform installer with a Python 3 (Windows). -# All logic lives in install.py so every OS runs one tested code path. Idempotent, safe to re-run. +# All logic lives in install.py so every OS runs one tested code path. +# It is idempotent and safe to re-run. # .\install.ps1 # $env:CLAUDE_HOME = "C:\path"; .\install.ps1 # override the target (testing) $ErrorActionPreference = "Stop" $here = Split-Path -Parent $MyInvocation.MyCommand.Path $script = Join-Path $here "install.py" -# Prefer launchers that are unambiguously Python 3. install.py and the hook use Python 3 syntax, so a -# bare `python` (Python 2 on some systems) is the last resort. +# Prefer launchers that are unambiguously Python 3. +# The installer and the hook use Python 3 syntax, so a bare `python`, which is Python 2 on some systems, is the last resort. if (Get-Command "py" -ErrorAction SilentlyContinue) { & py -3 $script @args } elseif (Get-Command "python3" -ErrorAction SilentlyContinue) { & python3 $script @args } elseif (Get-Command "python" -ErrorAction SilentlyContinue) { - # Verify a bare `python` is Python 3 before handing it Python 3 syntax - it is Python 2 on some setups, - # which would fail to parse install.py. py -3 and python3 above are Python 3 by construction. + # Verify a bare `python` is Python 3 before handing it Python 3 syntax, since it is Python 2 on some setups and would fail to parse install.py. + # The two branches above are Python 3 by construction, so only this one needs the check. & python -c "import sys; sys.exit(0 if sys.version_info[0] == 3 else 1)" 2>$null if ($LASTEXITCODE -ne 0) { Write-Error "Found python on PATH but it is not Python 3 (tried py -3, python3, python). Install Python 3." diff --git a/host-setup/agent-safety/install.sh b/host-setup/agent-safety/install.sh index d52c88d0..0d89e3d4 100755 --- a/host-setup/agent-safety/install.sh +++ b/host-setup/agent-safety/install.sh @@ -1,13 +1,14 @@ #!/usr/bin/env bash # Thin wrapper: run the cross-platform installer with a Python 3 (Linux / WSL / macOS / Proxmox). -# All logic lives in install.py so every OS runs one tested code path. Idempotent, safe to re-run. +# All logic lives in install.py so every OS runs one tested code path. +# It is idempotent and safe to re-run. # ./install.sh installs to ~/.claude # CLAUDE_HOME=/x ./install.sh overrides the target (testing) set -Eeuo pipefail here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# Pick the first candidate that is actually Python 3 - install.py and the hook use Python 3 syntax, so a -# bare `python` that is Python 2 must be rejected, not handed the script (it would fail on import). +# Pick the first candidate that is actually Python 3. +# The installer and the hook use Python 3 syntax, so a bare `python` that is Python 2 is rejected rather than handed the script, which would fail on import. py="" for c in python3 python; do if command -v "$c" >/dev/null 2>&1 && "$c" -c 'import sys; raise SystemExit(0 if sys.version_info[0] == 3 else 1)' 2>/dev/null; then diff --git a/scripts/README.md b/scripts/README.md index 219d37b0..5cfa0a37 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -30,9 +30,9 @@ Run it scoped to changed lines, matching the standing rule that existing prose i python3 scripts/prose_lint.py . --diff origin/develop ``` -Whole-tree (`python3 scripts/prose_lint.py .`) reports the legacy backlog as well, which is informational rather than a gate. `charset`, `dupword` and `spelling` are clean tree-wide, so CI gates those three and reports the rest warn-only. +Whole-tree (`python3 scripts/prose_lint.py .`) reports the legacy backlog as well, which is informational rather than a gate. `charset`, `dupword`, `spelling`, `comment-wrap` and `comment-case` are clean tree-wide, so CI gates those five and reports the rest warn-only. -The default rule set covers comment shape (`comment-wrap` and `comment-case`) alongside the prose rules. It did not, which meant a run nobody parameterized reported clean on a wrapped comment while the rule read as enforced, and comment shape is the most frequently regressed rule in agent-authored work. Reading the backlog it exposes needs no flag now, and gating it still needs `--diff`, because the tree carries several hundred of them. +The default rule set covers comment shape (`comment-wrap` and `comment-case`) alongside the prose rules. It did not, which meant a run nobody parameterized reported clean on a wrapped comment while the rule read as enforced, and comment shape is the most frequently regressed rule in agent-authored work. Reading the backlog it exposes needs no flag now, and gating it needed `--diff` while the tree carried several hundred of them. That backlog is cleared, so both comment rules gate whole-tree, and `--diff` is now about scoping a run rather than about surviving one. A wide scan skips the trees this repo generates rather than authors, currently `reports/`, which [`spec/audit.py`][audit] writes. A finding there is the audit engine's phrasing rather than an author's, so no edit to that tree can fix it, and leaving them in made the repo's own number mostly generated output. Naming such a path directly still reads it (`prose_lint.py reports`), so nothing becomes uncheckable. @@ -72,7 +72,7 @@ A comment sentence also has to start with a capital, which `comment-case` checks **A comment whose whole body is a URI is a reference rather than a sentence**, and neither rule applies to it. It cannot be capitalized or restructured without corrupting the address it exists to carry, so before the exemption every repo carrying a reference block inherited a finding no edit could answer. Consecutive reference lines are separate addresses rather than one sentence wrapping, which is why the exemption also stops the line below a URI from reading as its continuation. A URI inside a sentence is still prose, so the exemption requires the whole body to be the address and nothing else. -`charset` and `dupword` are clean tree-wide and gate CI. `charset-unknown`, `semicolon`, `dash`, `comment-wrap`, and `comment-case` run as one warn-only CI step, so the backlog is visible without blocking and is corrected as each file is next edited. +`charset`, `dupword`, `spelling`, `comment-wrap`, and `comment-case` are clean tree-wide and gate CI. `charset-unknown`, `semicolon`, and `dash` run as one warn-only CI step, so the remaining backlog is visible without blocking and is corrected as each file is next edited, or cleared in a deliberate batch. ## `repo_gate.py` diff --git a/scripts/prose_lint.py b/scripts/prose_lint.py index 37bb2eff..8653a032 100644 --- a/scripts/prose_lint.py +++ b/scripts/prose_lint.py @@ -670,7 +670,13 @@ def resume_at(carry: Carried, line: str) -> tuple[Carried, int | None]: # The initial guard anchors on a word boundary, so `J. Smith` reads as one name. # A sentence ending in an acronym such as CI is two sentences and has to be caught. # The second sentence may open in either case, since a lowercase opening is still a second sentence. -RUN_ON = re.compile(r'(? None: """Stripping the marker must not stop the rule seeing what follows it.""" self.assertEqual(['comment-wrap'], self.flag('a.sh', f'# 1. {self.RUN_ON}\n')) + def test_an_ellipsis_is_not_a_sentence_terminator(self) -> None: + """An ellipsis marks an elision inside one sentence, so its closing dot does not end one. + + Reading it as a terminator made a schematic comment two sentences, and the split the rule + then asked for would have broken the fragment the line exists to show. + """ + self.assertEqual([], self.flag('a.py', '# .editorconfig: [glob] ... end_of_line = lf\n')) + + def test_an_ellipsis_does_not_hide_a_real_run_on(self) -> None: + """The guard is one dot wide, so a terminator later on the line is still caught.""" + self.assertEqual(['comment-wrap'], + self.flag('a.py', '# Take the first ... and the rest. Another sentence.\n')) + + def test_an_ellipsis_before_a_question_or_bang_still_terminates(self) -> None: + """The guard covers the dot alternative only, since `?` and `!` after an ellipsis do end a sentence. + + Guarding the whole terminator class would have read these as one sentence, because the `?` and + the `!` are each preceded by the ellipsis' closing dot. + """ + for terminator in ('?', '!'): + with self.subTest(terminator=terminator): + self.assertEqual(['comment-wrap'], + self.flag('a.py', f'# Really...{terminator} Yes it does.\n')) + def test_a_step_marker_does_not_hide_a_lowercase_opening(self) -> None: """Before the strip, the digit read as the opening character, so comment-case never fired.""" self.assertEqual(['comment-case'], self.flag('a.sh', '# 1. deploy the hook.\n')) diff --git a/spec/audit.py b/spec/audit.py index 11e819d1..172bd368 100644 --- a/spec/audit.py +++ b/spec/audit.py @@ -171,7 +171,7 @@ def driftnote_findings(entry, spec, open_count): def repo_slug(entry): - # url is https://github.com// + # The url field is https://github.com// return "/".join(entry["url"].rstrip("/").split("/")[-2:]) @@ -186,8 +186,8 @@ def repo_selectors(entry, defaults): sel = set(entry.get("types", [])) sel.add(entry.get("workflowModel") or defaults.get("workflowModel") or "release") sel.add(entry.get("releaseTrigger") or defaults.get("releaseTrigger") or "two-phase") - # consumerModel has no defaults fallback - the registry schema does not allow defaults.consumerModel, and - # validate.py requires it on every cataloged repo. The guard only shields a malformed non-cataloged entry. + # The consumerModel field has no defaults fallback, since the registry schema does not allow defaults.consumerModel and validate.py requires it on every cataloged repo. + # The guard only shields a malformed non-cataloged entry. cm = entry.get("consumerModel") if cm: sel.add(cm) @@ -341,8 +341,8 @@ def title_and_intro(text): if s.startswith("## "): break region.append(ln.rstrip()) - # Trim the blank lines surrounding the region but keep the interior ones, so a paragraph-boundary - # difference is a real difference - the spec says the intro is copied verbatim. + # Trim the blank lines surrounding the region but keep the interior ones, so a paragraph-boundary difference is a real difference. + # The spec says the intro is copied verbatim. while region and not region[0]: region.pop(0) while region and not region[-1]: @@ -449,9 +449,8 @@ def check_interface(path, contract, text): tok = contract.get("artifactNameToken") if tok and tok not in code: findings.append(("DRIFT", f"interface: {path} missing the '{tok}-' artifact handoff")) - # Token checks only apply to a job that is present; an absent job is already reported by requiredJobKeys, - # so skip it rather than emit a redundant "missing token" for every token it cannot contain. Scan the - # job's code view so a token in a comment is not read as signal. + # Token checks only apply to a job that is present, since an absent job is already reported by requiredJobKeys, so skip it rather than emit a redundant "missing token" for every token it cannot contain. + # Scan the job's code view so a token in a comment is not read as signal. for job, toks in contract.get("requireTokensInJob", {}).items(): if job in jobs: block = _code_view(jobs[job]) @@ -467,15 +466,15 @@ def check_interface(path, contract, text): return findings -# A `uses: @<40-hex sha>` pin, plus only a trailing Dependabot version comment (` # v1.2.3` - the -# leading `v`-or-digit is required). Dependabot bumps both per repo, so that drift is governed (like EOL), not a -# fidelity deviation. Anchored to `uses:`, so a 64-hex docker digest and a tag/branch ref (`@v4`) do not match. +# A `uses: @<40-hex sha>` pin, plus only a trailing Dependabot version comment such as ` # v1.2.3`, where the leading `v` or digit is required. +# Dependabot bumps both per repo, so that drift is governed the way EOL is rather than being a fidelity deviation. +# It is anchored to `uses:`, so a 64-hex docker digest and a tag or branch ref such as `@v4` do not match. # Hex is case-insensitive, and a hand-written note on a pin is not version-shaped, so it survives to be compared. _ACTION_PIN = re.compile(r"(\buses:[ \t]*[^\s@]+)@[0-9a-fA-F]{40}(?:[ \t]+#[ \t]*v?[0-9][\w.\-]*)?") -# A workflow job's `needs:` list names the jobs it sequences after. In a verbatim job region a repo prunes that -# list to the targets it actually vendors - a `needs` entry naming an unvendored job fails the whole workflow to -# load - so the list is owned per repo, not fixed. Mask it (the inline `[ ... ]`, scalar, and block-list forms), -# same as the action pin. The interface contract still checks the required job keys separately. +# A workflow job's `needs:` list names the jobs it sequences after. +# In a verbatim job region a repo prunes that list to the targets it actually vendors, since a `needs` entry naming an unvendored job fails the whole workflow to load, so the list is owned per repo rather than fixed. +# Mask it in the inline, scalar and block-list forms, the same as the action pin. +# The interface contract still checks the required job keys separately. _JOB_NEEDS = re.compile( r"(^[ \t]*)needs:[ \t]*" r"(?:\[[^\]\n]*\]" # inline: needs: [a, b] (same line only) @@ -529,7 +528,8 @@ def git_file_history(rel_path): if rel_path in _HISTORY_CACHE: return _HISTORY_CACHE[rel_path] out = [] - # Decode as UTF-8/replace to match the downstream and canonical reads. A divergent decode would fabricate a mismatch. + # Decode as UTF-8 with replacement to match the downstream and canonical reads. + # A divergent decode would fabricate a mismatch. r = subprocess.run(["git", "log", "--format=%H", "--", rel_path], cwd=ROOT, capture_output=True, encoding="utf-8", errors="replace") if r.returncode == 0: @@ -548,8 +548,7 @@ def check_verbatim(label, down_text, canonical_rel, extract=None): byte diff is a hint to review, never proof of breakage. """ try: - # Same decode policy as the downstream copy and the git history, so a stray byte can never make - # otherwise-equal content hash differently across the three sources. + # Same decode policy as the downstream copy and the git history, so a stray byte can never make otherwise-equal content hash differently across the three sources. canon_text = (ROOT / canonical_rel).read_text(encoding="utf-8", errors="replace") except OSError: return [("DRIFT", f"verbatim: {label} canonical {canonical_rel} is unreadable from the hub (spec error?)")] @@ -609,8 +608,7 @@ def audit_repo(entry, spec, branch=None): if bool(entry.get("hasDevelop")) != dev_exists: findings.append(("DRIFT", f"registry: hasDevelop={entry.get('hasDevelop')} but develop {'exists' if dev_exists else 'is absent'}")) # Resolve the branch every content read is keyed on, reusing what the branch facts already read. - # An unresolvable one is an error rather than a run: every `?ref=` read would 404 and report the - # whole baseline as absent, which is a flood of letters describing the ref, not the repo. + # An unresolvable one is an error rather than a run, since every `?ref=` read would 404 and report the whole baseline as absent, which is a flood of letters describing the ref rather than the repo. # The branch facts above are reported either way, since they are already read and still true. ground_head = {"main": branch_main, "develop": branch_dev}.get(ground) if ground_head is None: @@ -618,22 +616,16 @@ def audit_repo(entry, spec, branch=None): if ground_head is None: return findings + [("ERROR", f"branch: ground-truth branch {ground} does not exist, so nothing could be read")], "" if main_exists and dev_exists: - # Commit counts mislead here: merge-commit promotions leave main permanently "ahead" while the - # head trees are identical, so tree equality is the no-drift fast path. When the head trees - # differ, empty compare files[] means develop is merely ahead (no main-side changes since the - # merge-base) - normal, no finding, no further API calls. + # Commit counts mislead here, since merge-commit promotions leave main permanently ahead while the head trees are identical, so tree equality is the no-drift fast path. + # Where the head trees differ, an empty compare files[] means develop is merely ahead, carrying no main-side changes since the merge-base, which is normal and yields no finding and no further API calls. if branch_main["commit"]["commit"]["tree"]["sha"] != branch_dev["commit"]["commit"]["tree"]["sha"]: cmp = gh(f"repos/{slug}/compare/develop...main", ok404=True) if cmp and cmp.get("files"): - # Non-empty files[] signals main-side changes, but is not usable directly: it is blind - # to cherry-picked promotions (develop may already hold identical content under - # different commit SHAs, e.g. promote/* branches) AND capped at 300 entries (#336). - # Instead, derive the main-side change set from the merge-base tree - paths whose - # object SHA (blob, or submodule pointer) differs base->main, additions and deletions - # included, no cap - then drop paths whose objects already match at develop: content - # develop already has is not "content develop lacks". Three recursive tree calls, so if - # any tree is truncated (or unexpectedly not a dict) the filter is skipped and the - # compare's unfiltered count kept (conservative, marked). + # A non-empty files[] signals main-side changes but is not usable directly. + # It is blind to cherry-picked promotions, where develop may already hold identical content under different commit SHAs such as a promote branch, and it is capped at 300 entries, per #336. + # Derive the main-side change set from the merge-base tree instead, taking paths whose object SHA, a blob or a submodule pointer, differs from base to main, additions and deletions included and with no cap. + # Then drop paths whose objects already match at develop, since content develop already has is not content develop lacks. + # That is three recursive tree calls, so where any tree is truncated, or unexpectedly not a dict, the filter is skipped and the compare's unfiltered count is kept, which is conservative and marked. trees = { "base": gh(f"repos/{slug}/git/trees/{cmp['merge_base_commit']['commit']['tree']['sha']}?recursive=1"), "develop": gh(f"repos/{slug}/git/trees/{branch_dev['commit']['commit']['tree']['sha']}?recursive=1"), @@ -685,15 +677,14 @@ def audit_repo(entry, spec, branch=None): # --- Secrets (names only) --- secrets = spec["secrets"] - # The codecov coverage requirement (the CODECOV_TOKEN secret and the codecov.yml file) is claimed by a - # type only at build profile: a lint-only language has no tests, so no coverage (spec/type-model.md). + # The codecov coverage requirement, meaning the CODECOV_TOKEN secret and the codecov.yml file, is claimed by a type only at build profile. + # A lint-only language has no tests and so no coverage, per spec/type-model.md. repo_profiles = entry.get("profiles", {}) if not isinstance(repo_profiles, dict): repo_profiles = {} coverage_active = any(secrets.get("typeMechanisms", {}).get(t) == "codecov" and repo_profiles.get(t) != "lint-only" for t in types) stores = {} - # No ok404: an empty store returns {"secrets": []}, so a 404/403 (permissions, rename) must - # surface as ERROR rather than cascade into false missing-secret DEFECTs. + # There is no ok404 here, since an empty store returns {"secrets": []}, so a 404 or 403 from permissions or a rename must surface as ERROR rather than cascade into false missing-secret DEFECTs. for store, path in [("actions", f"repos/{slug}/actions/secrets?per_page=100"), ("dependabot", f"repos/{slug}/dependabot/secrets?per_page=100")]: data = gh(path) stores[store] = {s["name"] for s in (data or {}).get("secrets", [])} @@ -706,9 +697,8 @@ def audit_repo(entry, spec, branch=None): for mech in claimed: for store in mech.get("stores", []): required_by_store[store] |= set(mech.get("requires", [])) - # Registry requiredSecrets[] are the domain-specific additions (STANDUP.md: requiredSecrets plus the - # implicit baseline). Mechanism-mapped names already carry their stores above, and unmapped ones are - # expected in the actions store and count as claimed (never stale). + # The registry requiredSecrets entries are the domain-specific additions, per STANDUP.md, being requiredSecrets plus the implicit baseline. + # Mechanism-mapped names already carry their stores above, and unmapped ones are expected in the actions store and count as claimed, never stale. required_by_store["actions"] |= set(entry.get("requiredSecrets", [])) forbidden = set(secrets["baseline"].get("forbids", [])) for mech in claimed: @@ -724,14 +714,13 @@ def audit_repo(entry, spec, branch=None): findings.append(("DRIFT", f"secrets: {name} in the {store} store is claimed by no applicable mechanism (stale?)")) # --- Dependabot ecosystem coverage --- - # A repo's tree implies Dependabot ecosystems it must track: github-actions when it ships workflows - # (the action versions they reference otherwise go stale, and a merge-bot then has no PRs to auto-merge), - # devcontainers when it ships a .devcontainer. dependabot.yml is YAML (no stdlib parser), so scan the - # declared package-ecosystem values by regex - anchored to the line start so a commented-out entry - # (# package-ecosystem: ...) is not read as declared. This asserts an implied ecosystem's *presence* - # only. Whether each declared ecosystem dual-targets main+develop (the fleet norm) is verified by - # inspection, not here. Only runs when dependabot.yml exists, since its absence is already a file-presence - # LETTER below. Language ecosystems (nuget/uv/npm) are directory-scoped and not yet cross-checked here. + # A repo's tree implies Dependabot ecosystems it must track, being github-actions where it ships workflows and devcontainers where it ships a .devcontainer. + # Without the first, the action versions those workflows reference go stale and a merge-bot then has no PRs to auto-merge. + # The dependabot.yml file is YAML and no stdlib parser reads it, so scan the declared package-ecosystem values by regex, anchored to the line start so a commented-out entry is not read as declared. + # This asserts an implied ecosystem's *presence* only. + # Whether each declared ecosystem dual-targets main and develop, which is the fleet norm, is verified by inspection rather than here. + # It only runs where dependabot.yml exists, since its absence is already a file-presence LETTER below. + # Language ecosystems such as nuget, uv and npm are directory-scoped and not yet cross-checked here. db = gh(f"repos/{slug}/contents/.github/dependabot.yml?ref={ground}", ok404=True) if db and db.get("content"): declared = set(re.findall(r'^[ \t]*-?[ \t]*package-ecosystem:[ \t]*["\']?([\w-]+)', base64.b64decode(db["content"]).decode("utf-8", "replace"), re.M)) @@ -746,11 +735,10 @@ def audit_repo(entry, spec, branch=None): findings.append(("DRIFT", f"dependabot: {eco} ecosystem not declared though {why}; add it for both main and develop per the fleet norm")) # --- File and section presence on the ground-truth branch --- - # appliesTo is matched against the repo's full selector set (types + workflowModel + releaseTrigger + - # consumerModel), so the release/operational develop ruleset is two data entries, not a code swap. - # Required sections union across same-path entries. A carried Markdown file must contain each heading - # scoped to this repo. A rename reads as missing and equivalence is judged by hand, so a missing section - # is DRIFT (a hint to verify), never a LETTER. + # The appliesTo selector is matched against the repo's full selector set, being types, workflowModel, releaseTrigger and consumerModel, so the release and operational develop rulesets are two data entries rather than a code swap. + # Required sections union across same-path entries. + # A carried Markdown file must contain each heading scoped to this repo. + # A rename reads as missing and equivalence is judged by hand, so a missing section is DRIFT, a hint to verify, and never a LETTER. sel = repo_selectors(entry, spec["registry"].get("defaults", {})) wanted_sections = {} # path -> set of required section names, unioned across applicable entries verbatim_secs = {} # path -> set of section names checked byte-for-byte against the hub canonical @@ -776,15 +764,15 @@ def audit_repo(entry, spec, branch=None): item = check_item.get(path) fid = item.get("fidelity") if item else "presence" if content is None: - # An interface unit's presence is DRIFT, not LETTER - a workflow's naming is more variable than a - # carried config, so absence is a hint to verify. Any other unit's absence is a file-presence LETTER. + # An interface unit's presence is DRIFT rather than LETTER, since a workflow's naming is more variable than a carried config, so absence is a hint to verify. + # Any other unit's absence is a file-presence LETTER. if fid == "interface": findings.append(("DRIFT", f"interface: {path} absent on {ground}, cannot verify its contract")) else: findings.append(("LETTER", f"file: {path} absent on {ground} (verify intent per AUDIT.md section 7)")) continue - # Guard on encoding, not truthiness: an empty file returns encoding "base64" with content "" (decode it - # to ""), whereas a too-large or non-inline payload returns encoding "none" (text stays None -> flagged). + # Guard on encoding rather than truthiness, since an empty file returns encoding "base64" with an empty content that decodes to an empty string. + # A too-large or non-inline payload returns encoding "none", where text stays None and is flagged. text = base64.b64decode(content["content"]).decode("utf-8", "replace") if content.get("encoding") == "base64" else None if path in ("README.md", "HISTORY.md") and text is not None: doc_texts[path] = text # retained for the README/HISTORY mirror check below @@ -805,35 +793,30 @@ def audit_repo(entry, spec, branch=None): findings.append(("DRIFT", f"verbatim: could not read {path} content on {ground} to compare (no inline content returned); verify by hand")) else: findings.extend(check_verbatim(path, text, item.get("reference") or path)) - # Heading-based presence is only meaningful for Markdown. A "section" named on a non-md file (e.g. a - # tasks.json task group) is an intent marker judged per AUDIT.md, not a heading grep. + # Heading-based presence is only meaningful for Markdown. + # A "section" named on a non-md file, a tasks.json task group being one, is an intent marker judged per AUDIT.md rather than a heading grep. needed = wanted_sections[path] verbatim_needed = verbatim_secs[path] if (needed or verbatim_needed) and path.endswith(".md"): if text is None: - # Fail loud rather than skip silently: the contents API returned no inline content (an - # oversized file, a symlink, a submodule), so the section check could not run - surface that - # instead of a false clean. + # Fail loud rather than skip silently, since the contents API returned no inline content, which happens for an oversized file, a symlink or a submodule. + # The section check could not run, so surface that rather than a false clean. findings.append(("DRIFT", f"section: could not read {path} content on {ground} to verify sections (no inline content returned); verify by hand")) else: present = heading_texts(text) for name in sorted(needed): if name.strip().lower() not in present: findings.append(("DRIFT", f"section: '{name}' not found as a heading in {path} on {ground} (renamed or missing; verify intent per AUDIT.md section 7)")) - # A verbatim section must match the hub's canonical byte-for-byte (EOL-normalized), like a - # verbatim file but scoped to the one `## ` region - so a universal rule block cannot - # drift or fall behind a newly added rule while its heading still passes the presence check. + # A verbatim section must match the hub's canonical byte-for-byte once EOL-normalized, like a verbatim file but scoped to the one `## ` region. + # A universal rule block then cannot drift or fall behind a newly added rule while its heading still passes the presence check. for name in sorted(verbatim_needed): findings.extend(check_verbatim(f"{path} section '{name}'", text, path, extract=lambda t, n=name: extract_section(t, n))) - # Undeclared-section advisory (spec/section-model.md): an H2 the manifest does not declare is a - # candidate duplicate of a verbatim section, or repo-specific content to relocate. Advisory only - - # a repo may legitimately carry its own project-specific sections (the AGENTS.md preamble allows - # them) - so it points at the reconciliation, it never fails. AGENTS.md and GOVERNANCE.md only, - # the two files whose section structure is governed by section-model.md. - # Skip the hub itself: its copies are the source and legitimately hold hub-only sections - # (e.g. Repository Onboarding and Conformance) that are deliberately not carried. A downstream - # repo carrying such a section is still flagged, which is the point. + # The undeclared-section advisory, per spec/section-model.md, treats an H2 the manifest does not declare as a candidate duplicate of a verbatim section, or as repo-specific content to relocate. + # It is advisory only, since a repo may legitimately carry its own project-specific sections, which the AGENTS.md preamble allows, so it points at the reconciliation and never fails. + # It covers AGENTS.md and GOVERNANCE.md only, the two files whose section structure is governed by section-model.md. + # Skip the hub itself, since its copies are the source and legitimately hold hub-only sections, Repository Onboarding and Conformance being one, that are deliberately not carried. + # A downstream repo carrying such a section is still flagged, which is the point. if path in ("AGENTS.md", "GOVERNANCE.md") and entry.get("name") != HUB_NAME: declared = {n.strip().lower() for n in (needed | verbatim_needed)} h2s = {ln[3:].strip().lower() for ln in text.splitlines() if ln.startswith("## ")} @@ -854,8 +837,8 @@ def audit_repo(entry, spec, branch=None): findings.append(("DRIFT", f"carried: {path} references the template repo by name or link outside its verbatim sections (the coordination flow is machinery this repo's readers should not see; state the behavior, not the destination)")) # --- HISTORY.md mirrors the README opening --- - # spec/readme-structure.md "HISTORY.md": the changelog opens as the README's twin - same H1 title and the - # same intro paragraph. Checked only when both files were readable (absence is already a file LETTER above). + # Per spec/readme-structure.md "HISTORY.md", the changelog opens as the README's twin, carrying the same H1 title and the same intro paragraph. + # It is checked only where both files were readable, since absence is already a file LETTER above. if "README.md" in doc_texts and "HISTORY.md" in doc_texts: r_title, r_intro = title_and_intro(doc_texts["README.md"]) h_title, h_intro = title_and_intro(doc_texts["HISTORY.md"]) @@ -864,10 +847,10 @@ def audit_repo(entry, spec, branch=None): elif r_intro != h_intro: findings.append(("LETTER", "history: HISTORY.md intro does not mirror the README intro - copy the README's opening paragraph (spec/readme-structure.md)")) - # --- README title/intro is the one canonical short description --- - # spec/readme-structure.md item 1 + GOVERNANCE.md "Repository Details": the H1 is the repo name, and the intro - # line after it is a link-free, <=100-char plain sentence that carries verbatim to the GitHub About - # description and (for a docker repo) the Docker Hub short description. The README is the source of truth. + # --- README title and intro are the one canonical short description --- + # Per spec/readme-structure.md item 1 and GOVERNANCE.md "Repository Details", the H1 is the repo name. + # The intro line after it is a link-free plain sentence of at most 100 characters that carries verbatim to the GitHub About description, and on a docker repo to the Docker Hub short description. + # The README is the source of truth. if "README.md" in doc_texts: title, intro = title_and_intro(doc_texts["README.md"]) intro_line = intro.split("\n")[0] @@ -902,9 +885,8 @@ def audit_repo(entry, spec, branch=None): findings.append(("LETTER", f"description: the Docker Hub short description ('{dh.strip()}') does not match the README intro ('{want}') - set it from the README (spec/readme-structure.md)")) # --- cspell single source of truth --- - # CODESTYLE.md "Markdown and Spelling": cspell.json is the one word list, and a cSpell words block left in - # a *.code-workspace duplicates it and silently drifts. Checked only when cspell.json is carried - its - # absence is already a file LETTER above, and a workspace list with no cspell.json is that same finding. + # Per CODESTYLE.md "Markdown and Spelling", cspell.json is the one word list, and a cSpell words block left in a *.code-workspace duplicates it and silently drifts. + # It is checked only where cspell.json is carried, since its absence is already a file LETTER above, and a workspace list with no cspell.json is that same finding. if gh(f"repos/{slug}/contents/cspell.json?ref={ground}", ok404=True) is not None: root_entries = gh(f"repos/{slug}/contents/?ref={ground}", ok404=True) or [] for it in root_entries: @@ -912,7 +894,7 @@ def audit_repo(entry, spec, branch=None): if not ws_name.endswith(".code-workspace"): continue ws = gh(f"repos/{slug}/contents/{ws_name}?ref={ground}", ok404=True) - # isinstance guard: the contents API returns a list for a directory, and .get would raise on it. + # The isinstance guard is needed because the contents API returns a list for a directory, where .get would raise. ws_text = base64.b64decode(ws["content"]).decode("utf-8", "replace") if isinstance(ws, dict) and ws.get("encoding") == "base64" else None if ws_text is None: findings.append(("DRIFT", f"cspell: could not read {ws_name} on {ground} to check for a duplicated word list; verify by hand")) @@ -922,8 +904,8 @@ def audit_repo(entry, spec, branch=None): # --- Registry driftNotes freshness --- findings.extend(driftnote_findings(entry, spec, len(findings))) - # Stamp the commit actually read for the ground-truth branch. Never fall back to another branch: - # a stamp naming develop while carrying main's sha would misattribute every finding. + # Stamp the commit actually read for the ground-truth branch. + # Never fall back to another branch, since a stamp naming develop while carrying main's sha would misattribute every finding. audited_sha = ground_head.get("commit", {}).get("sha", "") return findings, audited_sha @@ -975,9 +957,8 @@ def _selftest(): else: print(" ok split_jobs (inline-mapping job captured with its content)") - # Verbatim engine: EOL normalization, hashing, and the stale-vs-modified classification. Exercised here - # rather than only in production, because a latent bug in the comparison would otherwise surface as a - # false clean on a real fleet run. + # The verbatim engine, covering EOL normalization, hashing, and the stale-versus-modified classification. + # It is exercised here rather than only in production, because a latent bug in the comparison would otherwise surface as a false clean on a real fleet run. canon = "line one\nline two\nline three\n" verbatim_cases = [ # (label, down_text, canon_text, history, want) @@ -994,9 +975,8 @@ def _selftest(): if got != want: ok = False print(f" {'ok ' if got == want else 'FAIL'} want={str(want):>8} got={str(got):>8} verbatim: {label}") - # Action-pin neutralization: a Dependabot uses:@ bump (both the 40-hex sha and its ` # vN` comment) - # must not count as verbatim drift, but a changed action name must. This is what lets a verbatim workflow - # region survive routine action bumps while still catching a real fork. + # Action-pin neutralization, where a Dependabot uses:@ bump, meaning both the 40-hex sha and its ` # vN` comment, must not count as verbatim drift, while a changed action name must. + # This is what lets a verbatim workflow region survive routine action bumps while still catching a real fork. pin_a = " - uses: actions/checkout@" + "a" * 40 + " # v7.0.0\n" pin_b = " - uses: actions/checkout@" + "B" * 40 + " # v7.0.1\n" # uppercase hex + version bump pin_struct = " - uses: actions/setup-node@" + "a" * 40 + " # v7.0.0\n" @@ -1014,8 +994,7 @@ def _selftest(): else: print(" ok action-pin: version bump normalizes equal, changed action differs, hand-written note survives") - # needs-mask: a verbatim job region whose `needs:` list is pruned to the repo's vendored targets must not - # count as drift (the list is owned), but a structural change to the job's steps must. + # The needs-mask case, where a verbatim job region whose `needs:` list is pruned to the repo's vendored targets must not count as drift, since the list is owned, while a structural change to the job's steps must. needs_full = " github-release:\n needs: [get-version, validate-release, build-nugetlibrary, build-executable]\n runs-on: x\n steps: []\n" needs_pruned = " github-release:\n needs: [get-version, validate-release, build-executable]\n runs-on: x\n steps: []\n" needs_block = " github-release:\n needs:\n - get-version\n - build-executable\n runs-on: x\n steps: []\n" @@ -1041,9 +1020,8 @@ def _selftest(): print(" FAIL verbatim: forked github-release region should hash differently") else: print(" ok verbatim: a forked github-release region hashes differently from the canonical") - # Section-region extraction: the region includes the heading line, keeps a nested ### and a fenced ## inside - # the body, ends at the next sibling H2, is None if absent, and rehashes when the heading is re-cased - the - # per-section verbatim check depends on every one of these. + # Section-region extraction, where the region includes the heading line, keeps a nested ### and a fenced ## inside the body, ends at the next sibling H2, is None where absent, and rehashes where the heading is re-cased. + # The per-section verbatim check depends on every one of these. md = "# Title\n\n## Alpha\n\nbody a\n\n```\n## not a heading\n```\n\n### nested\nstill alpha\n\n## Beta\n\nbody b\n" a, b, gone = extract_section(md, "Alpha"), extract_section(md, "Beta"), extract_section(md, "Gamma") spaced = extract_section("## Alpha\n\nbody a\n", "Alpha") # extra marker-gap whitespace still locates @@ -1128,7 +1106,7 @@ def _selftest(): else: print(" ok cspell: workspace cSpell word list detected, a plain cspell.json mention is not") - # branch-drift direction split, covering modify/add/delete on main and a develop-only change + # The branch-drift direction split, covering a modify, add and delete on main plus a develop-only change. bd_base = {"keep": "a", "moda": "1", "modb": "2", "deld": "e", "devonly": "x"} bd_main = {"keep": "a", "moda": "9", "modb": "9", "add": "n", "devonly": "x"} # moved moda/modb, added 'add', deleted 'deld' bd_dev = {"keep": "a", "moda": "1", "modb": "7", "add": "m", "deld": "e", "devonly": "y"} # still at base on moda/deld, moved modb/add/devonly @@ -1139,9 +1117,8 @@ def _selftest(): else: print(" ok branch-drift: behind (modify/delete develop still at base) vs diverged (both moved), develop-only excluded") - # CLI parsing: a repo name and a flag value must not be confused for one another. The previous - # hand-rolled parse took every non `--` argument as a repo name, so `--branch develop` would have - # audited a repo called "develop" instead of overriding the branch. + # CLI parsing, where a repo name and a flag value must not be confused for one another. + # The previous hand-rolled parse took every non `--` argument as a repo name, so `--branch develop` would have audited a repo called "develop" rather than overriding the branch. cli_cases = [ ([], [], None, False, False), (["Utilities"], ["Utilities"], None, False, False), @@ -1328,8 +1305,8 @@ def main(argv=None): print(f"Not cataloged: {', '.join(sorted(missing))}", file=sys.stderr) return 2 - # Findings are a point-in-time snapshot. Stamp the run so anything derived from it (an onboarding - # issue, a report) carries its own freshness signal and a reader can tell whether it still applies. + # Findings are a point-in-time snapshot. + # Stamp the run so anything derived from it, an onboarding issue or a report, carries its own freshness signal and a reader can tell whether it still applies. run_utc = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") hub = subprocess.run(["git", "rev-parse", "--short", "HEAD"], capture_output=True, text=True, cwd=ROOT) hub_sha = hub.stdout.strip() if hub.returncode == 0 else "unknown" diff --git a/spec/fidelity_honesty.py b/spec/fidelity_honesty.py index 89de0a66..efb6343a 100644 --- a/spec/fidelity_honesty.py +++ b/spec/fidelity_honesty.py @@ -115,9 +115,9 @@ def fidelity_pass(spec): else: spread["differs"].append(r["name"]) spreads.append((unit, spread)) - # A verbatim candidate has NO hand-modified copy ("differs") and at least one confirmed match with - # the current canonical. Stale copies do not disqualify it - verbatim would flag them "stale -> - # re-vendor", which is the point. A unit that is entirely stale/unavailable is not confirmed uniform. + # A verbatim candidate has no hand-modified copy, meaning an empty "differs", and at least one confirmed match with the current canonical. + # Stale copies do not disqualify it, since verbatim would flag them as stale and owing a re-vendor, which is the point. + # A unit that is entirely stale or unavailable is not confirmed uniform. # An absent section does not disqualify it either, for the same reason an unavailable file does not. # Neither is evidence that a repo decided something locally, and only such evidence argues against promotion. # Treating absence as disqualifying would be the same conflation this bucket was split out to end. @@ -136,8 +136,8 @@ def manifest_gap_pass(spec, ref_repo): return None, [] slug = audit.repo_slug(entry) ground = entry.get("groundTruthBranch", "main") - # The git/trees endpoint takes a tree SHA, not a ref name, so resolve the branch to its tree SHA - # first (as audit.py does) - passing the branch name can 404 and silently drop the whole check. + # The git/trees endpoint takes a tree SHA rather than a ref name, so resolve the branch to its tree SHA first, as audit.py does. + # Passing the branch name can 404 and silently drop the whole check. # Fail loud on an unreadable reference adopter: an empty gaps list would report "none" (a false clean). br = audit.gh(f"repos/{slug}/branches/{ground}", ok404=True) if not br or "commit" not in br: @@ -190,15 +190,15 @@ def render_report(spreads, promote, gaps, ledger): spread_by_path = {e["path"]: sp for e, sp in spreads if sp is not None} def buckets(path): - # (still divergent, confirmed match, unavailable) repo sets for a unit. Unavailable is held apart - # from match: an absent copy cannot confirm a divergence was fixed. + # The three repo sets for a unit, being still divergent, confirmed match, and unavailable. + # Unavailable is held apart from match, since an absent copy cannot confirm a divergence was fixed. sp = spread_by_path.get(path) if not sp: return set(), set(), set() return set(sp["differs"]) | set(sp["stale"]) | set(sp["absent"]), set(sp["match"]), set(sp["unavailable"]) - # Keep only well-formed entries so --report degrades cleanly on a hand-malformed ledger instead of - # raising KeyError/TypeError downstream. validate.py reports the malformation loudly in CI. + # Keep only well-formed entries so --report degrades cleanly on a hand-malformed ledger rather than raising KeyError or TypeError downstream. + # The validate.py gate reports the malformation loudly in CI. dispositions = [d for d in ledger.get("dispositions", []) if isinstance(d, dict) and isinstance(d.get("path"), str) and isinstance(d.get("repos"), list) and isinstance(d.get("disposition"), str) @@ -212,8 +212,8 @@ def buckets(path): covered.setdefault(d["path"], set()).update(d["repos"]) # Untriaged: verbatim hand-modifications with no disposition, and live gaps with no gap disposition. - # Verbatim-only by design: an intent unit's byte diff is expected (judged by meaning), so only a verbatim - # hand-modification with no recorded disposition is a genuine anomaly worth surfacing. + # Verbatim-only by design, since an intent unit's byte diff is expected and is judged by meaning. + # Only a verbatim hand-modification with no recorded disposition is therefore a genuine anomaly worth surfacing. untriaged_files = [] untriaged_absent = [] for e, sp in spreads: @@ -338,8 +338,9 @@ def main(): if report_mode: content = render_report(spreads, promote, gaps, load_ledger()) - # CRLF to match the fleet default (reports/*.md is CRLF). Write bytes so the local platform does not - # re-translate. Git dates the file, so no timestamp is embedded (it would churn every regeneration). + # CRLF matches the fleet default, since reports/*.md is CRLF. + # Bytes are written so the local platform does not re-translate them. + # Git dates the file, so no timestamp is embedded, which would churn on every regeneration. (audit.ROOT / REPORT_PATH).write_bytes(content.replace("\n", "\r\n").encode("utf-8")) print(f"Wrote {REPORT_PATH} ({len(content.splitlines())} lines)") return 0 diff --git a/spec/validate.py b/spec/validate.py index 68e4cef9..e15a4cf8 100644 --- a/spec/validate.py +++ b/spec/validate.py @@ -13,13 +13,12 @@ ROOT = pathlib.Path(__file__).resolve().parent.parent -# Scope-selector vocabularies (see spec/scope-model.md), kept in sync with registry/repos.schema.json -# $defs. The four namespaces - project types plus these three - must stay disjoint, so a flat appliesTo -# token set in spec/files.json is unambiguous. +# Scope-selector vocabularies, per spec/scope-model.md, kept in sync with the $defs in registry/repos.schema.json. +# The four namespaces, meaning project types plus these three, must stay disjoint so a flat appliesTo token set in spec/files.json is unambiguous. WORKFLOW_MODELS = ("release", "operational") RELEASE_TRIGGERS = ("two-phase", "publish-on-merge", "dispatch-only", "none") CONSUMER_MODELS = ("push", "pull") -# How faithfully a carried unit is checked (spec/fidelity-model.md). Default presence. +# How faithfully a carried unit is checked, per spec/fidelity-model.md, defaulting to presence. FIDELITIES = ("presence", "intent", "verbatim", "interface") # The keys an interface unit's `contract` may carry (kept in sync with files.schema.json). CONTRACT_KEYS = {"requiredJobKeys", "requiredCheckName", "artifactNameToken", "requireTokensInJob", "forbidTokensInJob", "verbatimJobs"} @@ -58,8 +57,7 @@ def main(): target_mech = secrets["targetMechanisms"] mechanisms = secrets["mechanisms"] - # CI runs no JSON-schema validation, so shape-check secrets.json here to fail with a clear message - # rather than crash the cross-reference loops below. + # CI runs no JSON-schema validation, so shape-check secrets.json here to fail with a clear message rather than crash the cross-reference loops below. def check_secret_set(label, entry, need_kind): if not isinstance(entry, dict): errors.append(f"secrets.json: {label} is not an object") @@ -106,9 +104,8 @@ def check_secret_set(label, entry, need_kind): print(f" - {e}") return 1 - # defaults.workflowModel/releaseTrigger feed configure.sh's fallback and selector resolution, so an - # invalid value here breaks the apply or scopes wrong while every per-repo entry still validates - check - # them once. + # The defaults for workflowModel and releaseTrigger feed configure.sh's fallback and selector resolution. + # An invalid value there breaks the apply or scopes wrong while every per-repo entry still validates, so check them once. reg_defaults = repos.get("defaults", {}) default_model = reg_defaults.get("workflowModel") if default_model is not None and default_model not in WORKFLOW_MODELS: @@ -157,14 +154,13 @@ def check_secret_set(label, entry, need_kind): if model is not None and model not in WORKFLOW_MODELS: errors.append(f"{name}: workflowModel '{model}' invalid (expected {' or '.join(WORKFLOW_MODELS)})") - # releaseTrigger is a scope selector (spec/scope-model.md), so an invalid value would silently fail - # to match any releaseTrigger-scoped section rather than error. + # The releaseTrigger field is a scope selector, per spec/scope-model.md, so an invalid value would silently fail to match any releaseTrigger-scoped section rather than error. trigger = repo.get("releaseTrigger") if trigger is not None and trigger not in RELEASE_TRIGGERS: errors.append(f"{name}: releaseTrigger '{trigger}' invalid (expected one of {', '.join(RELEASE_TRIGGERS)})") - # consumerModel is a scope selector (spec/scope-model.md), so a cataloged repo must declare it or a - # push/pull-scoped section would fail open (never matched) on that repo. + # The consumerModel field is a scope selector, per spec/scope-model.md, so a cataloged repo must declare it. + # Otherwise a push-scoped or pull-scoped section would fail open on that repo, never matching. cm = repo.get("consumerModel") if cm not in CONSUMER_MODELS: errors.append(f"{name}: consumerModel '{cm}' invalid or missing (expected {' or '.join(CONSUMER_MODELS)})") @@ -172,10 +168,9 @@ def check_secret_set(label, entry, need_kind): eol = repo.get("lineEndings") if eol is not None and eol not in ("lf", "crlf"): errors.append(f"{name}: lineEndings '{eol}' invalid (expected lf or crlf)") - # An operational repo's endings follow the consuming app's platform, so they must be declared; a release - # repo omits the field and uses the fleet CRLF default. Resolve the effective model the same way - # configure.sh does (repo -> defaults -> release) so the requirement holds even if a repo relies on an - # operational defaults.workflowModel rather than setting it explicitly. + # An operational repo's endings follow the consuming app's platform, so they must be declared, where a release repo omits the field and takes the fleet CRLF default. + # Resolve the effective model the way configure.sh does, from the repo, then the defaults, then release. + # The requirement then holds even where a repo relies on an operational defaults.workflowModel rather than setting its own. effective_model = model or default_model or "release" if effective_model == "operational" and eol is None: errors.append(f"{name}: operational repo must declare lineEndings (lf or crlf)") @@ -196,25 +191,23 @@ def check_secret_set(label, entry, need_kind): errors.append(f"{name}: target '{target}' maps to undefined mechanism '{mech_key}'") continue spec_mech = mechanisms[mech_key] - # docker/static-secret must carry its required secrets + # A docker or static-secret target must carry its required secrets. for req in spec_mech.get("requires", []): if req not in required: errors.append(f"{name}: {target} requires secret '{req}' (missing)") - # oidc mechanisms must not carry a forbidden static key + # An oidc mechanism must not carry a forbidden static key. for bad in spec_mech.get("forbids", []): if bad in required: errors.append(f"{name}: {target} forbids secret '{bad}' (present)") - # mechanism label must match the target's expected mechanism family - # The repo's mechanism label (oidc / static-secret) must match the target mechanism's kind. - # An OIDC mechanism may still require a non-secret stored value (e.g. NUGET_USERNAME for - # NuGet/login), so requires-emptiness is not the signal - match on the explicit kind. + # The repo's mechanism label, oidc or static-secret, must match the target mechanism's kind. + # An OIDC mechanism may still require a non-secret stored value, NUGET_USERNAME for a NuGet login being one, so an empty requires list is not the signal. + # The match is on the explicit kind instead. kind = spec_mech.get("kind") if kind and mech != kind: errors.append(f"{name}: {target} labeled '{mech}' but its mechanism is '{kind}'") - # files.json appliesTo selectors must resolve to a known token, and no project type may collide with a - # reserved selector - a flat token set is only unambiguous while the namespaces stay disjoint. An - # unknown token fails open (it never matches), so a required file/section would silently apply nowhere. + # Every files.json appliesTo selector must resolve to a known token, and no project type may collide with a reserved selector, since a flat token set is only unambiguous while the namespaces stay disjoint. + # An unknown token fails open, never matching, so a required file or section would silently apply nowhere. reserved = set(WORKFLOW_MODELS) | set(RELEASE_TRIGGERS) | set(CONSUMER_MODELS) clash = known_types & reserved if clash: @@ -227,15 +220,14 @@ def check_selector(where, applies_to): return tokens = [] if applies_to == "*" else (applies_to if isinstance(applies_to, list) else [applies_to]) for tok in tokens: - # CI runs no JSON-schema validation, so guard the type here rather than crash on an unhashable - # token (e.g. a nested object) reaching the set-membership test below. + # CI runs no JSON-schema validation, so guard the type here rather than crash on an unhashable token, a nested object being one, reaching the set-membership test below. if not isinstance(tok, str): errors.append(f"files.json: {where} appliesTo has a non-string token {tok!r}") elif tok not in universe: errors.append(f"files.json: {where} appliesTo '{tok}' is not a known selector") - # CI runs no JSON-schema validation, so shape-check files.json here rather than crash on a malformed - # entry (a non-object baseline item, a non-array sections, a section that is neither string nor object). + # CI runs no JSON-schema validation, so shape-check files.json here rather than crash on a malformed entry. + # The shapes that reach this are a non-object baseline item, a non-array sections, and a section that is neither string nor object. files = load("spec/files.json") baseline = files.get("baseline", []) if not isinstance(baseline, list): @@ -251,9 +243,8 @@ def check_selector(where, applies_to): continue check_selector(path, item.get("appliesTo", "*")) - # fidelity governs how faithfully the unit is checked (spec/fidelity-model.md). CI runs no schema - # validation, so shape-check the fidelity fields here rather than let a malformed contract or an - # outside-root reference slip through and crash a later check. + # The fidelity field governs how faithfully the unit is checked, per spec/fidelity-model.md. + # CI runs no schema validation, so shape-check the fidelity fields here rather than let a malformed contract or an outside-root reference slip through and crash a later check. fid = item.get("fidelity", "presence") if fid not in FIDELITIES: errors.append(f"files.json: {path} fidelity '{fid}' invalid (expected one of {', '.join(FIDELITIES)})") @@ -301,9 +292,9 @@ def check_selector(where, applies_to): if not isinstance(elt.get("name"), str) or not elt.get("name"): errors.append(f"files.json: {path} section object missing a non-empty string 'name': {elt!r}") check_selector(f"{path} section '{elt.get('name', '?')}'", elt.get("appliesTo", "*")) - # A section may carry its own fidelity (intent default, or verbatim for a universal rule block - # checked byte-for-byte). verbatim is meaningful only on a Markdown file, where the heading - # delimits the region. The hub's own file is the canonical, so no reference is needed. + # A section may carry its own fidelity, defaulting to intent, or verbatim for a universal rule block checked byte-for-byte. + # Verbatim is meaningful only on a Markdown file, where the heading delimits the region. + # The hub's own file is the canonical, so no reference is needed. sfid = elt.get("fidelity", "intent") if sfid not in ("intent", "verbatim"): errors.append(f"files.json: {path} section '{elt.get('name', '?')}' fidelity '{sfid}' invalid (expected intent or verbatim)") @@ -312,11 +303,10 @@ def check_selector(where, applies_to): elif not isinstance(elt, str): errors.append(f"files.json: {path} section entry {elt!r} must be a string or object") - # Every declared section must resolve to a real `## ` in the hub's own copy of the file - # (spec/section-model.md "Enforcement"). Without this, a renamed or mistyped section name declares a - # region that does not exist: the downstream verbatim byte-match in audit.py then has nothing to - # compare, and the section silently stops being checked anywhere - the quiet-narrowing failure that - # Verification Discipline forbids. Markdown only, and only when the hub ships the file. + # Every declared section must resolve to a real `## ` in the hub's own copy of the file, per spec/section-model.md "Enforcement". + # Without this, a renamed or mistyped section name declares a region that does not exist. + # The downstream verbatim byte-match in audit.py then has nothing to compare, and the section silently stops being checked anywhere, which is the quiet-narrowing failure Verification Discipline forbids. + # This is Markdown only, and only where the hub ships the file. if path.endswith(".md") and (ROOT / path).exists(): hub_text = (ROOT / path).read_text(encoding="utf-8", errors="replace") headings = {m.group(1).strip() for m in re.finditer(r"^## (.+?)\s*$", hub_text, re.M)} @@ -325,16 +315,14 @@ def check_selector(where, applies_to): if isinstance(name, str) and name and name not in headings: errors.append(f"files.json: {path} declares section '{name}' but no '## {name}' heading exists in {path}") - # Validate the divergence ledger (spec/divergences.json) when present, so a mistyped repo name or - # disposition fails CI instead of silently dropping a burn-down row. + # Validate the divergence ledger in spec/divergences.json when present, so a mistyped repo name or disposition fails CI rather than silently dropping a burn-down row. dispositions = ("re-vendor", "track", "accepted", "upstream-candidate", "investigate", "retire") if (ROOT / "spec/divergences.json").exists(): div = load("spec/divergences.json") repo_names = {r.get("name") for r in repos["repos"] if isinstance(r, dict)} manifest_paths = {i.get("path") for i in baseline if isinstance(i, dict)} - # A verbatim section is an addressable unit too, labeled "path > section" (matches fidelity_honesty's - # SECTION_SEP), so a section-scoped divergence can carry its own disposition. Only well-formed section - # entries produce a label - a malformed one is already reported by the files.json checks above. + # A verbatim section is an addressable unit too, labeled "path > section" to match fidelity_honesty's SECTION_SEP, so a section-scoped divergence can carry its own disposition. + # Only well-formed section entries produce a label, since a malformed one is already reported by the files.json checks above. for i in baseline: if not isinstance(i, dict) or not isinstance(i.get("path"), str) or not isinstance(i.get("sections"), list): continue @@ -358,7 +346,7 @@ def check_selector(where, applies_to): errors.append(f"divergences.json: disposition {d!r} is not an object") continue p = d.get("path") - # isinstance guard first: a non-string path is unhashable and would crash the membership test. + # The isinstance guard comes first, since a non-string path is unhashable and would crash the membership test. if not isinstance(p, str): errors.append(f"divergences.json: disposition path {p!r} must be a string") elif p not in manifest_paths: @@ -380,7 +368,7 @@ def check_selector(where, applies_to): errors.append(f"divergences.json: gap {g!r} is not an object") continue gp = g.get("path") - # isinstance guard first: a non-string path is unhashable and would crash the membership test. + # The isinstance guard comes first, since a non-string path is unhashable and would crash the membership test. if not isinstance(gp, str): errors.append(f"divergences.json: gap path {gp!r} must be a string") elif gp in manifest_paths: