Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions .github/workflows/validate-task.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ jobs:
- name: Check repo gates step
run: python3 scripts/repo_gate.py

# 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.
# Every default prose rule is clean tree-wide, so each one gates rather than reporting a backlog.
# `charset-unknown` is clean too and still reports, for the reason the step below gives.
- name: Check prose step
run: python3 scripts/prose_lint.py . --check charset --check dupword --check spelling --check comment-wrap --check comment-case
run: python3 scripts/prose_lint.py . --check charset --check semicolon --check dash --check dupword --check spelling --check comment-wrap --check comment-case --check home-path

# 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, or cleared in a deliberate batch.
- name: Report prose backlog step
# A finding here names a character no tier covers, and classifying it is a fleet-law edit rather than a prose fix.
- name: Report unclassified characters step
continue-on-error: true
run: python3 scripts/prose_lint.py . --check charset-unknown --check semicolon --check dash --summary
run: python3 scripts/prose_lint.py . --check charset-unknown --summary
10 changes: 5 additions & 5 deletions OPERATIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 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:
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 eight of the nine and reports `charset-unknown` warn-only, where a bare run exits non-zero on any of the nine. `sentence-split` is in neither, so nothing below runs it and a local run reaches it only by naming it. Run the CI invocations:

```sh
python3 scripts/test_prose_lint.py
Expand All @@ -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 --check comment-wrap --check comment-case
python3 scripts/prose_lint.py . --check charset-unknown --check semicolon --check dash --summary
python3 scripts/prose_lint.py . --check charset --check semicolon --check dash --check dupword --check spelling --check comment-wrap --check comment-case --check home-path
python3 scripts/prose_lint.py . --check charset-unknown --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
```

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.
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. The second is that `sentence-split` is implemented and tested but named by no invocation, so nothing runs it.

Run the `editorconfig-checker` line before pushing a new file, and before pushing an existing file that a script rewrote rather than an editor. This repository defaults to CRLF and most tooling writes LF, so a new file fails that check on its first CI run rather than locally. A scripted rewrite is the same hazard on a file that was already correct, since reading and rewriting a whole file in text mode converts every line ending in it, which no prose or Markdown gate reports.

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.
The first prose invocation gates. The second reports a character that no tier covers, 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:

Expand Down
31 changes: 24 additions & 7 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,29 @@ One pull request, after a survey, deciding whether anything stands between this
- **Settled** - The reviewer counts the file and does not read it, rather than losing it earlier. The stated denominator equals the API's own `changedFiles` on **103 of 104** pull requests, the exception being one whose branch shrank between rounds.
- **Settled** - Splitting remains a real remedy for a feature branch and is unavailable for a promotion, whose head is `develop`, so a promotion carrying a partial round is a maintainer decision by construction.

### A Resolve the Loop Cannot Perform and a Thread Nobody Can Find

The review loop ends by replying on a thread and resolving it, and both halves failed on one pull request in ways the runbook describes nowhere. The resolve mutation was refused by the agent harness's own permission layer before any request left the machine, seconds after the reply mutation carrying the identical thread id had succeeded, so the refusal was neither GitHub's nor the id's. Handing the resolve to the maintainer then failed a second time, because the digest names a thread by its `PRRT_` node id, that id appears nowhere in the GitHub interface, and the person asked to resolve it could not find what to click.

**State** `ready`. **Touches** `scripts/pr_review.py`, the runbook section in [`.github/copilot-instructions.md`][copilot-instructions], and [`OPERATIONS.md`][operations]. **Cost** one pull request, since the query change is one field and the runbook change is one paragraph.

- **Carry a thread's own web address beside its node id, so a resolve can be handed to a person.** `Q_THREADS` selects `id`, `isResolved`, `path`, `line` and the first comment's `author` and `body`, and not its `url`, so the digest can name a thread and cannot point at it. Selecting `url` and printing it beside the id makes the hand-off one click.
- **Blocked by** - Nothing.
- **Checked** - `develop` at `0e4a1c2` on 2026-08-08, reading `Q_THREADS` in `scripts/pr_review.py` against the digest line that consumes it.
- **Detail** - The two identifiers are not interchangeable and neither is derivable from the other without a query. A `PRRT_` node id is what a mutation takes, and a `#discussion_r` fragment is what the web page anchors on.
- **Detail** - The evidence is [#620][pr-620], where a thread was handed over by node id and the reply was that it could not be found.

- **Give the runbook a shape for a write the harness refuses, which it currently has none for.** Its list of dead paths is entirely GitHub's own refusals, a silent no-op, a 422, and the wrong bot login for the API in use, so a local refusal matches none of them and reads as a bad identifier, which invites the retry a blocked write must never get.
- **Blocked by** - Nothing.
- **Checked** - `develop` at `0e4a1c2` on 2026-08-08, against the known-non-working-paths list in the runbook.
- **Detail** - The distinguishing evidence is that a reply on the same thread id, in the same session, had already succeeded and returned a comment url, so the identifier was demonstrably good.
- **Detail** - What cleared it was a permalink and a human click, and the durable remedy is a permission rule in host settings. That is host state rather than repo content, so it belongs in the runbook as a note rather than in a committed configuration file.

- **Confirm a resolve by re-reading the thread rather than by the mutation returning.** `reply` already exits 63 where the resolve did not report the thread resolved, which is the right shape, and a loop driving `gh api` by hand gets no exit code at all and so cannot notice. The rule worth writing down is that the state is the evidence.
- **Blocked by** - Nothing.
- **Checked** - `develop` at `0e4a1c2` on 2026-08-08, reading the exit-code table in the `scripts/pr_review.py` module docstring.
- **Detail** - This is the failure the suppressed-findings count already exists for, where a step that stopped running reads exactly like a step that passed.

## Standalone Chores

Small work with no research to preserve, selectable one bullet at a time.
Expand Down Expand Up @@ -418,12 +441,6 @@ Actions on issues that are the maintainer's to take, each carrying its evidence

Each was checked against the tree and has nothing left to do anywhere. Closing is the maintainer's call, and each wants the evidence quoted in the closing comment rather than a bare close.

- **[#519][issue-519], the hub's own tree does not pass the prose gate it ships.** Complete on the prose and on both questions.
- **Fixed by** - `f7a6a13` (snippets), `c9c92dd` (comments), `d791930` (hub-only Markdown), and the carried batch on `prose/carried-semicolons`.
- **Checked** - `develop` at `d791930` on 2026-08-07, where `python3 scripts/prose_lint.py --summary` reported 41 across 6 files, and 0 across 0 with the carried batch applied.
- **Closing evidence** - The whole-tree figure went 557 across 45 to zero, in four batches split by surface, being 184 in `catalog/snippets/`, 241 in non-Markdown comments, 90 in hub-only Markdown and 41 in the six carried files. Question 1 is answered by `reports/` being exempt as a generated tree, and question 2 by the snippets leading, since a non-conformant snippet seeds its violations into every repo that adopts it.
- **Closing evidence** - The issue's claim that the governance files were clean, and that this was therefore not a carry problem, was true of the checker of the day and false of the tree. Today's checker reports 38 findings against the same six files as they stood at `69688ec`, the commit the issue measured, while that commit's own checker reports zero. Scoping the list exemption to a sentence rather than a whole bullet accounts for 37 of the 38, because a colon anywhere ahead of the first semicolon had exempted every semicolon after it. The carry problem was real throughout and invisible, which is the stale-exemption hazard running in the loose direction.

- **[#557][issue-557], the agent-isolation rule and its two open questions.** Complete on the rule and on both questions.
- **Fixed by** - `9d85941`.
- **Checked** - `develop` at `9d85941` on 2026-08-06.
Expand All @@ -447,7 +464,6 @@ Each was checked against the tree and has nothing left to do anywhere. Closing i
[issue-483]: https://github.com/ptr727/ProjectTemplate/issues/483
[issue-489]: https://github.com/ptr727/ProjectTemplate/issues/489
[issue-509]: https://github.com/ptr727/ProjectTemplate/issues/509
[issue-519]: https://github.com/ptr727/ProjectTemplate/issues/519
[issue-521]: https://github.com/ptr727/ProjectTemplate/issues/521
[issue-523]: https://github.com/ptr727/ProjectTemplate/issues/523
[issue-550]: https://github.com/ptr727/ProjectTemplate/issues/550
Expand All @@ -464,6 +480,7 @@ Each was checked against the tree and has nothing left to do anywhere. Closing i
<!-- Pull requests -->

[pr-591]: https://github.com/ptr727/ProjectTemplate/pull/591
[pr-620]: https://github.com/ptr727/ProjectTemplate/pull/620

<!-- Upstream -->

Expand Down
6 changes: 3 additions & 3 deletions scripts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ uvx coverage@latest run --source=. -m unittest discover -s scripts && uvx covera

## `prose_lint.py`

Enforces the [`GOVERNANCE.md`][governance] "Documentation Style Conventions" rules that no linter checks: non-ASCII judged against the charset rule's three tiers, a semicolon in prose, a spaced hyphen joining or interrupting a sentence, a duplicated consecutive word, a British spelling, and the shape of a comment's prose.
Enforces the [`GOVERNANCE.md`][governance] "Documentation Style Conventions" rules that no linter checks: non-ASCII judged against the charset rule's three tiers, a semicolon in prose, a spaced hyphen joining or interrupting a sentence, a duplicated consecutive word, a British spelling, and the shape of a comment's prose. It carries one rule from elsewhere in that document, `home-path`, which comes from "Representative Data in Agent-Authored Text" and catches an absolute home path naming a real account. That rule closes the pattern-detectable sliver of its section and nothing beyond it, since the exposure the section exists for was name-shaped and no pattern finds a name. It is the one rule a checkout can turn off: an operational repository's runbook carries the literal path an operator types, so a run there drops `home-path` and says so on stderr rather than going quiet. A repository states its own model by which configuration payload it carries, and the hub carries both, so it reads as a release repository and the rule gates here.

The tiers decide by context rather than by a flat ban. Tier 1 carries no meaning its ASCII form loses and always flags. Tier 2 is an operator, kept next to a figure or another operator and replaced between words, so a threshold table reads as the range it is. Tier 3 is a unit or scientific symbol whose ASCII form would be a lie and never flags. Developer-typed characters such as emoji are preserved regardless of tier, and an un-tiered one is still reported as `charset-unknown` until it is classified.

Expand All @@ -30,7 +30,7 @@ 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`, `spelling`, `comment-wrap` and `comment-case` are clean tree-wide, so CI gates those five and reports the rest warn-only.
Whole-tree (`python3 scripts/prose_lint.py .`) reports zero, so a finding is a line the change under review wrote rather than backlog it inherited. CI gates every rule in the default set on that basis, except `charset-unknown`, which reports warn-only because classifying a character is a fleet-law edit rather than a prose fix.

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.
Comment thread
ptr727 marked this conversation as resolved.

Expand Down Expand Up @@ -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`, `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.
Every rule in the default set is clean tree-wide, which is what lets CI gate them over the whole tree rather than over a diff. `charset-unknown` reports instead of blocking, for the reason the tier discussion above gives.

## `repo_gate.py`

Expand Down
2 changes: 1 addition & 1 deletion scripts/test_prose_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -1406,7 +1406,7 @@ class TestChangedLines(unittest.TestCase):
The repo policy is that existing prose is corrected as each file is next edited rather than
swept, and this parse is the whole mechanism behind it. A parse that returns too little makes
the warn-only step silently stop reporting, and one that returns too much reports the backlog
a diff-scoped run silently stop reporting, and one that returns too much reports the backlog
as if the change introduced it.
"""

Expand Down