Skip to content

Make the prose gate reach comments, and carry it to the fleet - #520

Merged
ptr727 merged 12 commits into
developfrom
conformance/prose-gate-carry
Aug 2, 2026
Merged

Make the prose gate reach comments, and carry it to the fleet#520
ptr727 merged 12 commits into
developfrom
conformance/prose-gate-carry

Conversation

@ptr727

@ptr727 ptr727 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Comment shape is the most frequently regressed rule in agent-authored work, and this repo's answer to it was a rule with nothing running it. Two independent causes, both fixed here, plus the governance and host-setup items that came out of the same session.

Closes part of #519.

The gate did not check comments

comment-wrap and comment-case sat outside DEFAULT_RULES, so prose_lint.py . never checked them. That is why #519's whole-tree figure splits cleanly into dash and semicolon with no comment findings, and why the rule read as enforced while nothing ran it. Enabling them exposes 424 findings the tree already carried, worst in spec/audit.py (103) and host-setup/agent-safety/gh-write-guard.py (56).

A test now asserts a bare run catches a wrapped comment, so the rule cannot go quiet again.

The gate did not reach downstream repos

It is hub-only by design, so no fleet repo runs it. .github/actions/prose-gate lets a repo consume the rules from here instead of vendoring a 934-line script into twenty copies, which is possible only because this repo is now public: a public repository's composite actions are consumable by any repository, with no organization account required.

The ref design, which is the part worth reviewing:

  • The uses: pin is one literal, identical on both branches. uses: accepts no expressions, so a per-branch ref would be a permanent divergence that every develop -> main promotion has to preserve, fighting the branching model.
  • Branch-dependent behavior lives inside the action, where expressions are legal. A develop-targeted run reads the rules from hub develop, so an unpromoted rule change is exercised fleet-wide before it reaches main. Every other run uses the copy bundled at the pinned SHA, so a released repo's gate is reproducible and a hub commit cannot retroactively fail a re-run.
  • This satisfies GOVERNANCE.md "Action pinning" as written, since the downstream uses: is SHA-pinned. Dependabot bumps it like any other action pin, because the hub publishes real releases (latest 2.0.200) and the github-actions ecosystem resolves any GitHub-hosted action by owner/repo rather than through the marketplace.

Accepted trade: a hub develop commit can redden downstream develop CI fleet-wide. That is what the staging tier is for, and diff-scoping means only lines a change touches can fail.

Two exemptions, both because a finding named no possible edit

  • Generated trees are skipped when a wide scan expands into them, currently reports/. spec/audit.py writes it, so a finding there is the engine's phrasing rather than an author's. This answers prose_lint: the hub's own docs, spec, and catalog do not pass the gate it ships #519's first question, and it is why the hub's own number was 549 generated to 119 authored. Naming the path directly still reads it, so nothing becomes uncheckable.
  • An unpunctuated markdown HTML comment is a structural marker, not commentary. The reference-link group headers, the ToC-omit directive, and the agent-safety install markers are each matched verbatim by a tool, so capitalizing one or splitting an adjacent pair breaks what reads it. A markdown comment that does punctuate a sentence stays judged as prose, and the existing test asserting that still passes. This follows the precedent the URI exemption already set.

Governance and host-setup

  • "Commit" means commit and push. Nothing reviews a local commit: the Copilot loop, the required checks, and the maintainer all read the remote, so stopping at git commit reads as progress while no gate has run.
  • A Copilot decline carries proof, not a rationale. The command and its output, the code path that makes the concern impossible, or the rule that governs it. Disagreeing without evidence is not addressing a finding, so a thread is not resolved on one.
  • A low-confidence finding is not a low-value one. Copilot collapses the findings it is least sure of into the review body, where they reach no thread, and they are right the large majority of the time.
  • The authorization-scope backstop moved into the committed canonical. It was hand-written into one host's CLAUDE.md, which is the exact defect it describes: a rule that exists only where the history happens to be. install.py now syncs it, so it reaches a new machine. Three pre-existing spaced hyphens in that file are fixed in passing, matching the phrasing GOVERNANCE.md already uses.

Still outstanding on this branch

The catalog/snippets/** cleanup (#519's second question) is not in this commit. It is 184 findings across 17 files and lands as follow-up commits here rather than a separate PR. It is the highest-priority content fix, because a downstream repo copying build-release-task.yml inherits its 37 comment violations and is then flagged for lines it did not write.

Verification

test_prose_lint.py 148 tests, test_repo_gate.py 23, test_pr_review.py 27, spec/audit.py --selftest, scripts/repo_gate.py, spec/validate.py, and markdownlint all pass. prose_lint.py . --diff origin/develop reports zero on every line this change touches.

Comment shape is the most frequently regressed rule in agent-authored
work, and nothing was enforcing it. Two causes, both fixed here.

The rules sat outside DEFAULT_RULES, so a run nobody parameterized
reported clean on a wrapped comment while the rule read as enforced.
Enabling them exposes 424 findings the tree already carried.

The gate is hub-only, so no downstream repo runs it at all. A composite
action lets a repo consume the rules from here rather than vendoring a
934-line script into twenty copies, which is possible now that this repo
is public. The pin is one literal, identical on both branches, because
`uses:` takes no expressions and a per-branch ref would diverge at every
promotion; the branch-dependent rule source lives inside the action,
where expressions are legal, so a develop-targeted run reads hub develop
and every other run uses the copy bundled at the pinned SHA.

Also skip generated trees on a wide scan, since a finding in reports/ is
the audit engine's phrasing and no edit can fix it, and treat an
unpunctuated markdown HTML comment as a structural marker rather than
commentary, because a tool matches each one verbatim.

Add the host-wide authorization-scope backstop to the committed
canonical so it reaches a new machine, state that "commit" means commit
and push, and raise a Copilot decline from a rationale to evidence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 18:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens the fleet prose gate by ensuring comment-shape rules are enforced by default, adds targeted carve-outs for generated output and structural markdown markers, and introduces a reusable composite action so downstream repos can consume the gate logic from this hub repo.

Changes:

  • Include comment-wrap and comment-case in DEFAULT_RULES, with tests preventing regressions.
  • Skip generated reports/ content during wide scans (while still allowing direct scans of that tree), and treat unpunctuated markdown HTML comments as structural markers.
  • Add a composite action (.github/actions/prose-gate) to run the diff-scoped gate using either bundled rules (pinned) or hub develop rules for develop-targeted runs.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
TODO.md Adds a backlog item about cross-repo workflow reuse and link refs needed by that addition.
scripts/test_prose_lint.py Adds regression tests for default comment-shape coverage, generated-tree skipping behavior, and markdown-marker handling.
scripts/README.md Documents the updated default rules, generated-tree behavior, and markdown marker carve-out.
scripts/prose_lint.py Enables comment-shape rules by default; implements generated-tree skipping and markdown-marker carve-out logic.
host-setup/agent-safety/claude-md-safety.md Moves the authorization-scope backstop into the committed host-safety doc and fixes formatting.
GOVERNANCE.md Clarifies “commit” includes pushing, and strengthens the evidence requirement when declining review findings.
.github/actions/prose-gate/action.yml New composite action to run the prose gate downstream with pinned or develop-sourced rules.

Comment thread .github/actions/prose-gate/action.yml Outdated
The canonical was rewritten with LF over its CRLF, which is the
whole-file ending flip the line-endings rule warns about, so restore it
and keep the diff to the lines the change actually touches.

Pin composite action metadata to LF alongside the workflows, since
Dependabot rewrites the `uses:` pins inside it the same way.

Split the paths input into an array and pass it quoted, so a value
holding a glob character scans what it names rather than what the
pattern matches.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 18:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/actions/prose-gate/action.yml:72

  • read -ra scan <<<"$PATHS" only reads up to the first newline, so a downstream workflow that provides paths as a multi-line YAML string (common for readability) will silently ignore everything after the first line. Since the intent is to split on whitespace, normalize newlines before read so all paths are honored.
        set -Eeuo pipefail
        # Split on whitespace into an array, so several paths stay several arguments.
        # Passing the array quoted keeps each element literal.
        # An unquoted expansion would instead glob, and a path holding `*` would scan the match.
        read -ra scan <<<"$PATHS"
        python3 "$SCRIPT" "${scan[@]}" --diff "$BASE"

.github/actions/prose-gate/action.yml:55

  • The curl fetch of prose_lint.py has no retry/backoff, so transient network failures can fail the gate even when the ref/script is correct. Adding a small retry policy makes CI more reliable without changing the trust model (a non-200 still fails due to -f).
          dst="$RUNNER_TEMP/prose_lint.py"
          url="https://raw.githubusercontent.com/ptr727/ProjectTemplate/$ref/scripts/prose_lint.py"
          # Fail loudly rather than skipping the gate, since a silent skip reports a clean pass.
          curl -fsSL "$url" -o "$dst"
          echo "script=$dst" >>"$GITHUB_OUTPUT"

The first fleet adoption surfaced three defects, all in how the gate
behaves when it cannot scope itself.

An unusable diff widened to the whole tree, which reported the
repository's entire backlog as though the change had introduced it. That
turned PhotoCleaner's first run into 420 findings its branch never
touched. Scoping to nothing instead would report a false clean, so
neither default is honest and the run now fails naming the cause, with
an exit code distinct from a findings exit.

The action assumed a pull_request event, but a fleet repo gates on push,
so `base_ref` is always empty there and every run fell through to the
pinned rules. Read the branch name when `base_ref` is absent, and pin
only main, since every other branch merges into develop and should
exercise its rules before promotion.

The action also now checks the base resolves before scanning, so a
shallow checkout or a misspelled ref fails naming itself rather than
silently changing what the run measures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 18:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/prose_lint.py:146

  • discover() is intended to be scoped by the user-provided paths, but the git-backed branch currently calls tracked_paths(root) without applying any pathspec/prefix filtering. Since tracked_paths() runs git ls-files with no pathspec, passing a directory like docs/ will still include the entire repo (and depending on tracked_paths() path joining semantics, may also produce incorrect paths when root != '.'). Consider always listing from the repo root and then filtering by the requested root prefix so paths reliably scopes the scan.
        root = p if p.is_dir() else Path('.')
        tracked = tracked_paths(root)
        if tracked is None:
            print(f'warning: git cannot describe {root}, falling back to a filesystem walk',
                  file=sys.stderr)

@ptr727

ptr727 commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Answering the suppressed finding from the review on 4e21a27, since a suppressed comment reaches no thread and the merge gate counts it as outstanding either way.

scripts/prose_lint.py:146 - discover() does not scope by paths. Disproven. The premise is that tracked_paths() runs git ls-files with no pathspec, and it does not: it runs git -C <root> ls-files -z, and -C is the scoping mechanism, changing the command's working directory so git ls-files lists only that subtree.

$ git -C docs ls-files | wc -l
6
$ git ls-files | wc -l
113

Scoping through the full discover() path, including the nested case:

$ python3 scripts/prose_lint.py docs --list-files | wc -l
6
$ python3 scripts/prose_lint.py docs --list-files | grep -vc '^docs/'
0
$ python3 scripts/prose_lint.py catalog/snippets/workflows --list-files | wc -l
13
$ python3 scripts/prose_lint.py catalog/snippets/workflows --list-files | grep -vc '^catalog/snippets/workflows/'
0

The second half of the concern, that paths may be reconstructed incorrectly when root != '.', is disproven by the same output. return [root / name ...] rejoins each name to the root, so the emitted paths are docs/content-import.md rather than a bare content-import.md, which is also what makes the --diff path match them against git diff output. An absolute root works too, emitting absolute paths.

Worth noting the alternative it proposes, listing from the repo root and filtering by prefix, would be a behavior change rather than a fix: git -C resolves the root through git's own discovery, so it works from a subdirectory of the repo and against a path outside the current one, where a root-relative prefix filter would not.

No code change. The scoping the finding asks for is what the code already does.

Four rounds of Copilot's suppressed findings went unanswered across
three pull requests in one day, each found by the maintainer rather than
by the tooling. Three causes, all fixed here.

The digest read suppressed blocks only from reviews on the current head,
treating "superseded by a push" as "answered". A finding nobody replied
to left the digest the moment the branch moved, and the run then
reported zero. It now reports every round, marking which are earlier, so
the reader decides whether an older finding is moot.

A review body that reads "generated no new comments" routinely carries a
collapsed block of findings directly beneath that sentence, and the
summary line, the review decision and an empty thread list all agree
that such a review is clean. Say so in the rules, and require an answer
that quotes the finding, carries a verdict per finding and states the
count, since a suppressed finding has no thread to pair an answer to and
is otherwise unverifiable.

The action read its paths input with `read -ra`, which stops at the
first newline, so a multi-line paths block scanned only the first entry
and reported the rest clean. Fold newlines first, report what is
scanned, and retry a transient fetch failure rather than failing a
correct change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 19:34
@ptr727

ptr727 commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Suppressed findings answered

Answering the collapsed findings from review 4839451158 on 4ad0156, Suppressed comments (2). Both correct, both fixed in 90d81f7. A suppressed finding has no thread, so each is quoted here to pair the answer to it.

1. .github/actions/prose-gate/action.yml:72 - Fixed in 90d81f7

read -ra scan <<<"$PATHS" only reads up to the first newline, so a downstream workflow that provides paths as a multi-line YAML string (common for readability) will silently ignore everything after the first line.

Correct, and worse than it reads: a silent under-scan is the one failure a gate must never have, because it reports the unscanned paths as clean. Reproduced:

$ PATHS=$'.\ndocs\nscripts'
$ read -ra scan <<<"$PATHS"; echo "${scan[*]} (${#scan[@]})"
. (1)

Newlines are now folded before splitting, and the step prints what it scanned so an under-scan is visible in the log rather than silent:

$ read -ra scan <<<"$(printf '%s' "$PATHS" | tr '\n' ' ')"; echo "${scan[*]} (${#scan[@]})"
. docs scripts (3)

An empty result after parsing now fails the step rather than scanning nothing.

2. .github/actions/prose-gate/action.yml:55 - Fixed in 90d81f7

The curl fetch of prose_lint.py has no retry/backoff, so transient network failures can fail the gate even when the ref/script is correct. Adding a small retry policy makes CI more reliable without changing the trust model (a non-200 still fails due to -f).

Agreed, including the reasoning about the trust model. Now curl -fsSL --retry 3 --retry-delay 2 --retry-all-errors, so -f still fails the step on a non-200 and only the transient case is retried.


Previously answered from review 4839478321 on 4e21a27, Suppressed comments (1): the discover() scoping claim, Disproven, with evidence in this comment. That reply predates this convention and paraphrases the finding rather than quoting it, which is the gap this convention closes.

3 of 3 suppressed findings on this PR are now answered.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

scripts/prose_lint.py:929

  • main() now returns exit code 2 when --diff cannot be resolved, but the module header still states “Exit 1 if any violation is found”, and callers may be relying on the documented 0/1 contract. Please update the top-level documentation (and any call-site docs) to mention the new non-findings error exit code.
    scope = changed_lines(a.diff) if a.diff else None
    if a.diff and scope is None:
        # Widening to the whole tree answers a different question, and answers it silently.
        # A caller scoping to a change gets the backlog reported as though the change made it.
        # A CI adoption hits this first, where an unresolvable base walls off the first run.

scripts/pr_review.py:126

  • digest() now counts suppressed findings across all Copilot review rounds, not just the current head. scripts/README.md still says the digest “covers the reviews on the current head only”, which is now incorrect and may mislead users interpreting suppressed= / earlier= output.
    # Every round, not just the head, because a suppressed finding has no resolved state to read.
    # Head-scoping treated "superseded by a push" as "answered", and the two are not the same.
    # A finding nobody replied to left the digest the moment the branch moved, reporting zero.
    # That is how four rounds went unanswered across three pull requests in one day.
    # The head is still marked per block, since a finding on an older round may be moot.

Comment thread scripts/prose_lint.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.github/actions/prose-gate/action.yml:21

  • rules-ref input description doesn’t match the implemented selection logic. The step selects develop for any run where TARGET != "main" (including feature branches and push runs), not only “a develop-targeted run”. This can mislead downstream consumers about when the floating develop ref is used.
  rules-ref:
    description: >-
      Hub ref supplying the rules.
      Empty selects develop for a develop-targeted run and the pinned action version otherwise.
    required: false

The check read each file's filesystem path, so a directory above the
checkout decided it. A repository cloned under a parent named reports
carried that parent into every file's parts, which read the whole scan
as deliberately requested and put the repository's own generated tree
back into an ordinary sweep.

Ask git where the scan root sits inside its repository and judge the
path from there, so nothing outside the checkout can reach the decision.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 19:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (3)

scripts/prose_lint.py:177

  • explicit.add(q) is a leftover after the generated-tree filtering change, but explicit is never read. Dropping this write avoids implying there’s special handling for “explicit” discovered files.
                found.append(q)
            elif not GENERATED_TREES.isdisjoint(Path(prefix).parts):
                # The root named is itself inside a generated tree, so it was asked for.
                found.append(q)
                explicit.add(q)

.github/actions/prose-gate/action.yml:94

  • The paths input is parsed into an argv array, but it’s still passed before --diff with no -- sentinel. If a workflow accidentally includes a token that starts with - (e.g. --list-files), argparse will treat it as an option and the scan can be unintentionally altered or short-circuited. Pass options first and include -- so every token is treated as a literal path.
        echo "Scanning ${#scan[@]} path(s): ${scan[*]}"
        python3 "$SCRIPT" "${scan[@]}" --diff "$BASE"

scripts/prose_lint.py:156

  • explicit is created and populated but never read. This adds dead state to the discovery logic and makes it harder to tell what inputs are actually treated as “explicit”. Remove the unused set and its first write (the direct-file case) to keep discover() minimal.

This issue also appears on line 173 of the same file.

    found: list[Path] = []
    explicit: set[Path] = set()
    for raw in paths:
        p = Path(raw)
        if p.is_file():

The gate passed its paths input before the options with no sentinel, so
a token beginning with a dash parsed as an option. A paths value holding
 turned the gate into a file listing that exits 0, which
is a silent pass. Pass the options first and terminate them with .

The explicit set became dead when the generated-tree decision moved to
the repository-relative path: it was written three times and read none.
Remove it rather than leave state implying a filter that no longer runs.

The rules-ref description still said develop is selected for a
develop-targeted run, where the logic selects it for every branch except
main.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 19:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/prose_lint.py:36

  • DEFAULT_RULES now includes comment-wrap/comment-case, but OPERATIONS.md still states that a bare python3 scripts/prose_lint.py … run omits those checks (OPERATIONS.md:9). That runbook will mislead readers about what a default run covers and when comment-shape findings should appear.
DEFAULT_RULES = frozenset({'charset', 'charset-unknown', 'semicolon', 'dash', 'dupword',
                           'spelling', 'comment-wrap', 'comment-case'})

scripts/pr_review.py:160

  • If a review node has a null/empty commit.oid (which the code already handles elsewhere), the digest currently prints raised on , earlier round, which is hard to interpret. Use an explicit placeholder like unknown so the SUPPRESSED marker stays readable when the SHA is missing.
        sha = ((n.get('commit') or {}).get('oid') or '')[:8]
        where = 'on head' if sha == head[:8] else f'raised on {sha}, earlier round'
        lines.append(f'  SUPPRESSED ({where}): no thread to resolve, '
                     'answer it in the PR conversation quoting the finding')

GraphQL returns a null commit for a pending or partial review, and the
digest rendered that as "raised on , earlier round". The sha is the one
thing that traces a suppressed finding back to the round that raised it,
so an empty one reads as a formatting glitch rather than as a finding
still needing an answer.

Say the commit is unknown and that it counts as outstanding, since an
unknown round is not a reason to drop a finding.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 21:43
@ptr727

ptr727 commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Suppressed findings answered

One from review 4839761898 on 87052e8, Suppressed comments (1). Correct, fixed in 27cb352.

scripts/pr_review.py:160 - Fixed in 27cb352

When a suppressed-finding block comes from a review node that lacks a commit OID (e.g., a pending/partial review), sha becomes empty and the output renders as SUPPRESSED (raised on , earlier round), which is confusing and makes it harder to trace what round raised the finding. Consider emitting an explicit "unknown commit" marker when the commit OID is missing.

Correct, and reproduced against a payload with 'commit': None:

SUPPRESSED (raised on , earlier round): no thread to resolve, ...

The finding is sharper than a formatting nit. The sha is the only thing tracing a suppressed finding back to the round that raised it, since there is no thread to open, so an empty one reads as a rendering glitch rather than as something still needing an answer. That is the same shape as the defect this whole change set exists to fix, a finding present but not legibly present.

It also has precedent in the file: live_state already guards a null commit, with a case named test_a_null_author_or_commit_does_not_raise explaining that GraphQL returns null for a deleted account and that a crash there stalls the wait. I added a second reader of the same field without carrying that guard across.

Now commit unknown, treat as outstanding, and the block still counts toward suppressed=N, since an unknown round is not a reason to drop a finding. A case covers both the marker and the count.


9 of 9 suppressed findings on this PR are now answered.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/prose_lint.py:970

  • In --diff mode, the repository-mismatch guard runs after discover() has already enumerated files. On large trees this makes an invocation that will immediately error still pay the full discovery cost even though the result is discarded. Consider running the repo_root mismatch check before discover() (optionally keeping the current --list-files bypass) so the failure is fast and side-effect free.
    rules = set(a.checks or DEFAULT_RULES)
    files = discover(a.paths or ['.'], tuple(a.exclude))

    if a.list_files:
        for f in files:
            print(rel(f))
        return 0

    if a.diff:
        # `git diff` runs in the current directory while the paths may name another checkout.
        # Scanning one repository and diffing another intersects to nothing.
        # The run then reports clean, which is the false clean this gate exists to prevent.
        # It cost a real verification once, where a branch read zero from the wrong directory.
        here = repo_root(Path('.'))
        for raw in (a.paths or ['.']):
            there = repo_root(Path(raw))
            if here and there and here != there:
                print(f'error: --diff resolves against {here}, but {raw} is in {there}. '
                      'Run the gate from the repository being scanned, since a diff taken '
                      'elsewhere scopes every finding away and reports a false clean.',
                      file=sys.stderr)
                return 2

The guard ran after discovery, which reads every tracked file to
classify it as text, so a run it was always going to reject paid that
cost and threw the result away.

Check it first. `--list-files` stays exempt, since it reports the scan
scope and never consults the diff.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 21:50
@ptr727

ptr727 commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Suppressed findings answered

One from review 4839783461 on 27cb352, Suppressed comments (1). Correct, fixed in 36e140c.

scripts/prose_lint.py:970 - Fixed in 36e140c

In --diff mode, the repository-mismatch guard runs after discover() has already enumerated files. On large trees this makes an invocation that will immediately error still pay the full discovery cost even though the result is discarded. Consider running the repo_root mismatch check before discover() (optionally keeping the current --list-files bypass) so the failure is fast and side-effect free.

Correct. The guard consumes nothing from discover(), so its position was accidental rather than chosen, and discovery is not cheap: it runs git ls-files and then reads the first block of every tracked file to classify it as text. A rejected run paid all of that and discarded it.

The parenthetical is the part worth crediting. --list-files should keep the bypass, and for a reason rather than for compatibility: it reports the scan scope and never reaches changed_lines, so a diff taken elsewhere cannot mislead it. Listing files across repositories is a legitimate thing to ask for, and the guard exists to stop a false clean, which listing cannot produce. The check is now if a.diff and not a.list_files.

Verified the reorder changed only the cost and not the behavior: the real misuse still errors with the same message, an ordinary run still reports normally, and a new case asserts discover is never called on the rejected path (disc.assert_not_called()) with a second case covering the --list-files bypass across repositories.


10 of 10 suppressed findings on this PR are now answered.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment thread scripts/prose_lint.py
The guard tested for a different repository root, so a path under none
at all passed it. That case fails the same way and more quietly:
discovery walks the filesystem, every absolute key misses the diff's
repository-relative ones, the scope drops every file, and the run exits
zero over an unscanned tree.

Require the same root instead, which covers both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 21:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment thread scripts/prose_lint.py Outdated
The generated-tree fallback rebuilt a bare filename, which dropped the
directories the decision reads. It errs toward scanning, so the effect
was noise rather than a missed finding, but the safe direction was
accidental and a later edit could invert it.

Say it instead: with no repository-relative path there is nothing to
judge, so the file is scanned. Skipping on doubt is how a gate reports
clean over what it never read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 22:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit cbd5eb3 into develop Aug 2, 2026
7 checks passed
@ptr727
ptr727 deleted the conformance/prose-gate-carry branch August 2, 2026 22:43
ptr727 added a commit that referenced this pull request Aug 3, 2026
…ckup-event tooling to main (#528)

Promotes four commits to `main`. Two of them change **carried** files,
so downstream repos read the stale text until this lands, which is what
makes the promotion the delivery step rather than bookkeeping.

## What this delivers to the fleet

**Carried rule text**, picked up by every repo on its next re-vendor:

| PR | File and section | Change |
| --- | --- | --- |
| #526 | `AGENTS.md`, Context and Delegation Discipline (**verbatim**) |
A wait separates three outcomes and says which one it reached: run the
command in the foreground before backgrounding it, never let `\|\| echo
'[]'`, `\|\| true` or `2>/dev/null` stand in for a failure, emit on
failure, and bound the wait. The session rule also stops ending a
session on a third review round, which read as license to leave a loop
open while it was still producing defects |
| #526 | `GOVERNANCE.md`, Verification Discipline (**verbatim**) | A
launched process is not a result, and a cause nobody observed is not a
diagnosis |
| #526 | `GOVERNANCE.md`, PR Review Etiquette (**verbatim**) | Every
finding ends in one of five actions rather than at a round count: fixed,
disproven with proof the reviewer can read, deferred against a filed
issue, declined with the maintainer's explicit answer, or fixed as a
class where the code keeps earning it |
| #520 | `CODESTYLE.md` and the prose gate | The comment rules the gate
now reaches, carried to the fleet through a public composite action |
| #526, #527 | `.github/copilot-instructions.md` | A quota or rate-limit
answer is terminal rather than pending; a request pending with no pickup
is a third state with a recovery recipe; three corrections below |

**Runbook corrections**, each one a path an agent followed to a wrong
answer this week:

- `gh pr view --json reviewRequests` **omits a Bot reviewer entirely**,
reporting an empty set while Copilot sits in it. This is how a live
stall was misdiagnosed to the maintainer as no request having been made.
- Removal was called impossible for want of a named mutation.
`requestReviews` **replaces** the reviewer set when `union` is false,
which is the clear half of the recovery, and it resolved a real
thirteen-and-a-half-hour stall in 35 seconds.
- The reviewer login has a **third** spelling. A timeline
`review_requested` carries login `Copilot` with type `Bot`, against
GraphQL's `copilot-pull-request-reviewer` and the `[bot]` suffix REST
user objects add. A filter keyed to either documented form selects
nothing there.

**Hub-only tooling** (`scripts/`, not carried): `pr_review.py` gains
exit `40` for a reviewer answer that carries no commit and exit `50` for
a request nothing picked up, with the window and interval guards those
needed. `prose_lint.py` and the gate queue changes from #520 and #522
ride along.

## Why the rules moved

Three stalls in one day, each reported as waiting on the reviewer, none
of them that. A CI watcher whose command did not exist on the installed
`gh` and whose fallback turned every error into "nothing yet". A stall
explained afterwards with a throttle that appears nowhere in the record.
And a review request that was pending while nothing acted on it. The
common shape is a wait that cannot tell "not yet" from "never", and a
report of patience standing in for a reading nobody took.

## Verification

`scripts/test_pr_review.py` 59 pass, `scripts/test_prose_lint.py` 153
pass, `scripts/test_repo_gate.py` 23 pass, `spec/audit.py --selftest`
pass, `spec/validate.py` clean, `repo_gate.py` clean, both
`prose_lint.py` invocations clean, `editorconfig-checker` clean. Both
source pull requests were driven to a clean Copilot round: #526 over
seven rounds and ten findings, #527 over four rounds and five findings,
every one of the fifteen real and answered.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
ptr727 added a commit to ptr727/PhotoCleaner that referenced this pull request Aug 3, 2026
* Consume the fleet prose rules from the hub

The prose rules that govern this repository live in the hub, and until
now nothing here ran them, so a comment or a sentence breaking a
documented rule reached main with every linter green.

Consume the hub's composite action rather than vendoring its checker, so
a rule change lands in one place instead of in every repository holding
a copy. A develop-targeted run reads the rules from hub develop, so an
unpromoted change is exercised here before it is promoted, and every
other run uses the copy bundled at the pinned commit so a released build
stays reproducible.

The gate reports only lines a change touches, so the repository's
existing prose backlog blocks nothing and is corrected as each file is
next edited.

Checkout gains full history in the lint job, because diffing against the
base branch needs that branch present.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Point the prose gate at the branch it merges into

The gate runs on push, because this repository has no pull_request
trigger, so there is no event base to read and the first run diffed
against an empty ref. Unresolvable, it reported the whole repository
instead of the lines this change touches.

Name the base explicitly as the branch being merged into, and skip main,
which only receives promotion merges already gated on develop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Skip the prose gate on a publish run

A publish reaches validate-task through build-release-task, where the
content was already gated when it was pushed. Re-reading it there would
diff the whole unpromoted delta against main and could fail a release on
prose that already passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Cut the publish-run comment to one sentence per line

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Describe what this workflow does, and mark the pin temporary

The comment described the action's main-run behavior, which this
workflow never reaches because it skips main outright. A maintainer
reading it would assume a prose gate runs on main.

The pin targets an unmerged hub commit, and that intent lived only in
the pull request description, which does not survive the merge. State it
inline instead, including that Dependabot cannot bump a pin resolving to
no tag, so the repoint is manual.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Only pay for full history on the runs that use it

The lint job always full-cloned, including publish runs where the prose
gate is skipped, which is time and network a release does not need. Tie
the fetch depth to the same condition the gate uses, so the two cannot
drift apart.

Repoint the pin to the hub branch head, which carries the fix for a
multi-line paths input that scanned only its first entry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Quote the fetch depths so the true branch is not falsy

An unquoted 0 is falsy in an Actions expression, so the ternary
collapsed to 1 whenever the condition held and every run shallow-cloned,
leaving the prose gate no base branch to diff against.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Cut the wrapped comment sentences to one per line

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Repoint the prose gate at the hub commit that carries it

The pin named the head of an unmerged hub branch, which would have gone
unreachable once that branch was squashed and deleted, breaking this
repository's gate later with nothing here to explain it.

ptr727/ProjectTemplate#520 has landed, so point at the commit on hub
develop instead. It still carries no release tag, so Dependabot cannot
bump it yet and the note says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Describe the pin as a SHA rather than as a branch

A pin that calls itself develop stops being true the moment develop
moves. Name what it contains instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727 added a commit that referenced this pull request Aug 11, 2026
…'s Directory (#666)

`prose_lint.py` chose whether to run `home-path` by asking whether the
**current directory** was an operational repository, rather than the
path it was asked to scan.

Stand in an operational repo, scan a release repo, and the check is
discarded. The skip prints to stderr and the run exits 0.

That silences the one rule written because real paths carrying family
names reached a **public** comment. A gate that switches itself off
based on where the caller happens to stand is answering a different
question than the one it was asked — and answering it as a pass.

## Reproduced, and measured against a real violation

A release repo carrying `/home/someuser/project/output.log`, scanned
from an operational checkout:

**Before** — silent skip, exit 0, violation missed:

```
note: home-path is not checked in an operational repository, where an absolute
path is the operator instruction rather than observed data.
exit=0
```

**After** — found, exit 1, and identical to what running from inside the
repo gives:

```
README.md:3: home-path: absolute home path '/home/someuser' -> use a constructed path, not an observed one
exit=1
```

## The fix

The scanned path decides. Paths spanning two repositories **refuse**
rather than pick one, since each declares its own workflow model and no
single rule set can be right for both. A path git cannot place falls
back to itself rather than to `.`, which would quietly put the caller's
directory back in charge of the verdict.

The `--diff` same-root guard is untouched and still correct — `git diff`
genuinely does run in the current directory. But it only ever ran
**under `--diff`**, so it never covered this path at all.

## Why the existing suite missed it

`TestOperationalExemption` already covers the exemption in five cases.
It mocks `repo_root` to return one value for every argument, which
cannot tell the caller's repository from the scanned one — the exact
distinction the defect lives in.

The four new cases set those two roots to **different models**. Three of
them fail against the previous code. The fourth guards the fallback
against a future regression rather than reproducing the defect, and I am
flagging that rather than counting it as four.

## Provenance

Reported by the **ESPHome-Config** agent, from a symptom I could not
reproduce — its scenario exits 2 here on the guard that landed in #520.
I went looking anyway and found this adjacent defect, which is real.

Independently hit by the **HomeAutomation-Config** agent within the
hour, which had run the gate from a scratch directory and got a clean
result worth nothing. It re-ran from inside the checkout and found 190
violations in prose it was about to submit.

Two agents, two different wrong directories, same hour. The tool gave
both of them a pass.

## Verification

Full local suite green: all five self-tests, `audit --selftest`,
`gh-write-guard --selftest`, `repo_gate.py`, `prose_lint.py` tree-wide,
editorconfig-checker.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727 added a commit that referenced this pull request Aug 11, 2026
…ed (#668)

Two false cleans reported by the ESPHome-Config agent, both reproduced against `develop` before any fix, and both members of one class: **a run that reads nothing prints what a run with nothing to report prints.**

## The two reports, reproduced

Constructed repositories, one seeded finding each. Verdicts are pre-fix.

| invocation | pre-fix | post-fix |
| --- | --- | --- |
| `prose_lint.py . --diff BASE` | 1 finding, exit 1 | unchanged |
| `prose_lint.py /abs/path --diff BASE`, same directory, same repository, same ref | **silent, exit 0** | identical to the relative form |
| new file, untracked, the whole change | **silent, exit 0** | 2 findings, exit 1 |
| the same bytes, staged | 2 findings, exit 1 | unchanged |

The absolute-path run returned absolute paths from discovery while the diff named repository-relative ones, so the intersection was empty. The same-root guard could not fire, because the run genuinely was in the right repository.

The untracked hole is not a `--diff` quirk. `git ls-files` omits an untracked file exactly as `git diff` does, so a whole-tree sweep passed over it for its own reason, which retires the workaround `OPERATIONS.md` documented.

## The invariant

Every input to a verdict is read from the repository being scanned, and none of them from the directory the process happens to stand in. That covers the rule set, the file set, the diff, and the keys joining the last two. #666 established it for the rule set alone; the other three still read the working directory.

- One `repo_key` helper puts both sides in repository coordinates. It replaces three hand-rolled idioms, one already correct and two not, and that disagreement was the defect.
- Discovery reads tracked plus untracked-and-not-ignored; the diff counts an untracked file as added in full. Ignored paths, generated trees and binaries stay out.
- The diff is taken at the scan root, so `diff.relative` cannot re-anchor it and a subdirectory run works rather than exiting 2 with advice to move.
- **The class fix**: every run states its scope on stderr, including a clean one.

```text
scope: 4 of 117 file(s) read, 344 changed line(s), diff against 'HEAD'
scope: 117 file(s) read, whole tree
```

All five known false cleans exit 0 in silence. Per-route guards only ever close the route somebody thought of, and the sixth is found by a reviewer or not at all.

## One deliberate removal

The #520 guard refusing a scan of one repository while standing in another is gone. It existed because the diff was taken where the process stood; anchoring the diff on the scan root is what it was approximating, so the case is answered rather than turned away. `repo_prefix` goes with it as dead code. Its two cases are replaced: a path under no repository is still refused, now by the diff itself, and scanning one repository from another is asserted to diff the one scanned.

## Verification

Twelve new cases build real git repositories rather than mocking `repo_root` and `discover`, because a mock supplies the join that was broken. **Eight of the twelve fail against the pre-fix source**; the four that pass are exclusion cases, and each was checked rather than assumed redundant. One, a `diff.relative` case, passed because both sides were anchored on the process's directory and agreed by accident, and its docstring now claims only that.

- 221 self-tests, prose gate, `repo_gate` (eol, eol-coverage, sha-pin), `spec/validate.py`, markdownlint over 45 files, editorconfig-checker: all clean locally.
- `--list-files` byte-identical over this repository, 117 files.
- Timings within noise: 0.22s to 0.24s diff-scoped, 1.45s to 1.39s whole-tree.
- CRLF verified byte-wise on all three touched Markdown files.

`TODO.md` line 409 named the wrong-directory false clean as an open objection to running doc gates in the pre-commit hook; that objection no longer applies and the entry says so.

## Answered in review

Five rounds. After round 1 every finding arrived suppressed rather than as a
thread, and five of the seven were real.

- **A subtree argument was pinned** after a finding read `git ls-files` as
  returning repository-root-relative names under `-C`. It does not, measured on
  git 2.51, and the proposal would have discarded the narrowing the path
  argument asks for. The first version of that test passed under the proposal
  too, so it now asserts the discovered count rather than the findings.
- **The fallback walk is now gated on `repo_root(base)`**, which is a behaviour
  change beyond the description above. `tracked_paths` answers None both for a
  tree git cannot describe and for one holding no tracked files, and only the
  first justifies a walk. Read as emptiness, a subtree of new files took the
  walk, which applies no ignore rules, and scanned an ignored build output while
  printing that git could not describe a tree git describes fine. The
  conflation predates this branch; untracked files joining the file set is what
  made it reachable.
- **Three docs and two docstrings** claimed the file set is what git tracks plus
  what it is not ignoring, with no qualifier, over a fallback that consults git
  not at all. Each now says where the ignore rules apply.
- **A cited test count was wrong as well as brittle**, 210 rather than 209, and
  now carries the commit it was measured at.
- **One finding is disproven**: `contextlib.chdir` raises no Python floor here,
  since this module already called `enterContext` in fourteen places.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants