Conversation
The validation workflow reads Markdown, workflows, EditorConfig, prose and the JSON spec, and it does not read shell at all. Four scripts are in the tree today, and a host-setup layer of standalone bash is arriving, so the one language here with no code-layer gate is the one about to grow. `prose_lint.py` already covers `.sh` comments, which makes the gap precise: the comment layer is gated and the code layer is not. ## Two changes **A `shellcheck` step**, pinned `koalaman/shellcheck:stable`, run through docker beside the EditorConfig step it resembles. The file list comes from `git ls-files` rather than a glob, so a script added later is gated without editing the step. **`scripts/test_host_gate.py` added to the self-test step.** It ran in no workflow. The step listed five entries and the host-gate tests were not among them, so 61 cases covering the tighten-only overlay merge were never executed by CI. The comment above that step reads *"Each gate in `scripts/` is proven by a case that reintroduces the fault it catches"*, which was true of four gates out of five. Not run here: `host_gate.py` itself, which measures a host against the fleet floors — a runner is a different machine with different needs, and the layering logic is the part worth testing. ## What turning the gate on costs One disable. `repo-config/configure.sh` holds a jq program that must stay single-quoted, because `$in` and `$k` are jq's variables and shell expansion would empty both. Double-quoting to satisfy SC2016 is the defect the quoting prevents. The file already carries four SC2016 disables in exactly that style, so this is the fifth and the convention is completed rather than introduced. ## Verification The gate is proven able to fail, not merely observed passing: - **The disable is load-bearing.** Stripping it returns the finding; restoring it clears. - **A new script is caught.** Adding a file with an unquoted expansion takes the list from four entries to five and fails the step with SC2034, SC2086 and SC2154. Removing it passes. - **The corpus is otherwise clean.** All four existing scripts pass unmodified. - Full local suite green: shellcheck, all six self-tests, actionlint, `repo_gate.py`, `prose_lint.py`, editorconfig-checker. Landing this before the host-setup scripts is deliberate. A gate retrofitted after the code arrives has its first exercise as an argument about someone's work, and the disable here would be reviewed buried in a large move rather than on its own merits. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…'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>
…ler Ignored (#665) Rolling the safety kit across a fleet needs an answer to *"is this machine current"*, and there was none. [#365](#365) tracks machines by tick, a tick records that someone ran something once, and the kit has changed three times since the earliest one. Its own body admits this: *"a machine ticked below is not necessarily current."* Verifying meant typing three `grep` commands per host and reading them by eye. ## The stamp Every install writes `~/.claude/agent-safety-stamp.json` and prints one pasteable line: ``` server-2 | Debian GNU/Linux 13 (trixie) | hub 31880f0 | payload a1ea65e4e604308f | agent-safety v1, fleet-bootstrap v1 | 2026-08-11T00:48:58Z ``` Host name, host type, the hub commit it came from, a digest of the bytes installed, the marker versions actually found in `CLAUDE.md`, and when. ## `--report` Read-only. Run it from a fresh hub checkout and it compares that machine against that checkout: - **CURRENT** — matches, exit 0 - **STALE** — with the specific reasons, exit 1 - **NOT INSTALLED** — no stamp, exit 2 It returns before creating anything, so reporting on a clean machine leaves it clean. ## Three judgement calls worth stating **Compared on the payload digest, not the commit.** A machine installed from an older commit whose kit bytes never changed *is* current, and calling it stale sends someone to re-run an installer that would write the same file byte for byte. **The digest covers the installed bytes, not the tree state.** A clean commit and a dirty checkout install different content under the same SHA. A dirty install is recorded as dirty, because a stamp that claims a commit identifies the bytes when it does not is the thing this exists to prevent. **Blocks are read back off disk.** Not assumed from what the installer meant to write. A block edited or deleted by hand since the install reports stale, and a start marker without its end does not count as present — the half-written case a presence check reads as success. ## A defect fixed in passing `main()` took no arguments while both wrappers pass `"$@"` through, so every flag was silently discarded. `install.py --help` performed a full install instead of printing usage. It now parses, and an unknown flag exits 2 having changed nothing — asserted by comparing the stamp before and after. ## Verification `test_install.py`, 13 cases, each proving a verdict by reintroducing the state it reports: - Report on a never-installed machine says so **and creates nothing** - A changed payload reports stale; a hand-deleted block reports stale - **The printed remedy actually clears it** — re-running returns CURRENT, so the verdict is not a dead end - A half-written block is not counted as present - `--bogus` is rejected with the stamp unchanged; `--help` prints usage and installs nothing - **Every file the kit deploys is in the digest**, checked in both directions: changing each listed file moves the digest, and every file `install.py` reads is in the list All cases run against a throwaway `CLAUDE_HOME`, never the invoking user's. Wired into the self-test step, because a test that runs nowhere is the defect [#664](#664) just found in another gate. ## Merge order This touches the same self-test block as [#664](#664). Whichever merges second needs a one-line rebase in that list — I will handle it. Follow-up not in scope here: the issue body still carries the manual grep instructions. I will rewrite it to the one-command-per-platform form once this lands, so the instructions describe a flag that exists. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…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>
`docs/host-setup.md` said which tools a host must provide, deliberately named no installer, and promised that per-platform install commands were tracked separately. Nothing tracked them. A machine with a fresh operating system had no path to the state the contract describes, so every host was stood up by hand against a document that only measured the result. This is the missing half. ## What Lands `host-setup/linux/` holds three tools. `install-tools.sh` manages git, gh, jq, git-restore-mtime, node, python, uv and dotnet, taking each from the distribution where the distribution keeps up and from upstream where it does not. `upgrade-host.sh` separates upgrading the packages of the current release from moving to the next release, and refuses a release move on Proxmox and on any distribution it does not recognize. `setup-github.sh` configures the key, git, and commit signing, gating on the two registrations that happen in a browser and verifying each against the key lists GitHub publishes rather than asking whether the human did it. `host-setup/bootstrap.sh` is the one file fetched on its own, because a host with no git and no checkout is what it exists to fix. It resolves a ref to the commit it names, downloads that revision as a tarball, and hands control to one entry point inside it. A tarball rather than a clone, since a clone needs git on a host that may not have it, and a resolved commit cannot be stale. ## The Decisions Worth Reviewing **The ref is first class rather than an escape hatch.** `AUDIT.md` and the sync procedure both tell an agent to fetch this repository immediately before reading it, so a loader that could only reach `main` could not serve a repository testing a hub change before it promotes. Every run prints the ref it was given and the commit that resolved to, before it does anything. **Nothing here needs Python, and that is a boundary rather than an accident.** Requiring an interpreter to upgrade a package or install a tool would make the first step of standing a host up depend on the thing that step exists to provide. The Python floor is a development requirement, meaning `scripts/` and `spec/`, and a host that only runs services never has to meet it. **The gate and the tooling are joined at code time and nowhere else.** `scripts/host_gate.py` measures a host against the floors and `host-setup/` changes one, and neither calls the other in either direction. A host set up by hand years ago is an ordinary host, so the gate reports what it is missing and running this tooling is a remedy a person chooses. `scripts/test_bootstrap.py` asserts the one connection that does belong, which is that every tool the spec requires on Linux is one this tooling can install. **`GOVERNANCE.md` gains scope rather than an exception.** The rule that reaching this repository means a checkout governs a tool that reads hub content, and a loader reads none, so it sits outside that rule instead of being excused from it. The bound is content rather than caller, which makes it testable: a loader references no path inside the tree it fetches except the single entry point it hands control to. An exception invites widening and a scope does not. **The three scripts under `linux/` share no helper file.** Each is independently fetchable and runnable, which is the property that lets a host with no checkout use one without the others, and a shared file would take it away the moment one sourced a sibling. About thirty lines each of logging, the dry-run wrapper, the confirmation prompt and a temporary directory are duplicated, identically rather than merely similarly, and `host-setup/README.md` records that as deliberate so nobody helpfully factors it out. ## Verified by Running, Not by Review Every gate in `validate-task.yml` passes over this branch, run locally as the workflow runs it: shellcheck over all eight tracked scripts, the eight self-tests, prose lint undiffed over the whole tree, the repo gates, markdownlint over 46 files, actionlint, editorconfig, and cspell. The three tools were exercised on Debian 12 and 13, Ubuntu 24.04 and 26.04, Proxmox, and both WSL distributions. Each reports identically on a second run on the same host, and installing twice reports nothing changed the second time. The loader resolves, downloads, extracts, and refuses a ref whose tree carries no tooling with a message naming what it did not find, which is what `main` produces today and becomes the handoff when this lands. Each test in `scripts/test_bootstrap.py` was watched to fail before it was trusted, by reintroducing the fault it catches. ## Wiring `scripts/test_bootstrap.py` is added to the self-test step by name rather than by placement, per the defect that step's own history recorded. It sits in `scripts/` rather than beside `bootstrap.sh` because it is a read-only gate over the spec and the tooling, which is what that directory is chartered for, where `host-setup/` is chartered as the layer that writes to the machine. The comment above that step counted three `host-setup/` entries where two are `host-setup/` and one is `spec/`. It now states the placement rule instead of a count, so adding an entry does not falsify it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR promotes a set of previously-reviewed fixes and new tooling that make repository gates more reliable (especially prose_lint.py diff-scoped runs) and adds host bootstrap/setup scripts plus CI wiring so these checks and tools are continuously exercised.
Changes:
- Fix
prose_lint.pyscan scoping by anchoring rule selection, file discovery (tracked + untracked), diffs, and diff/file keying to the repository being scanned; also emit an explicit “scope read” note on every run to prevent silent false-cleans. - Add host bootstrap/setup tooling under
host-setup/(loader + Linux setup scripts) and add self-tests that pin the loader invariant and spec/tooling coverage. - Expand CI validation to include shellcheck across all tracked
.shfiles and run additional self-tests (bootstrap + agent-safety installer tests).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | Updates backlog notes to reflect the corrected diff-scoping behavior and records a follow-up item for installer error-handling semantics. |
| scripts/test_prose_lint.py | Adds/adjusts test coverage for scan-root selection, diff/file keying invariants, and historical false-clean regression cases. |
| scripts/test_bootstrap.py | New self-test asserting the bootstrap loader “single entrypoint” invariant and that Linux tooling covers required spec tools. |
| scripts/README.md | Documents the updated prose_lint.py discovery and scoping model and the “always report scope” invariant. |
| scripts/prose_lint.py | Implements repo-anchored keying/scoping, includes untracked files in discovery and diff scope, and prints a scope summary on every run. |
| repo-config/configure.sh | Adds a documented shellcheck suppression for the jq filter’s $in/$k variables. |
| OPERATIONS.md | Updates operational guidance to match new discovery + diff-scoping semantics and scope reporting. |
| host-setup/README.md | New documentation for host setup layout, bootstrap usage, and the deliberate “no shared helpers” rule for Linux scripts. |
| host-setup/linux/upgrade-host.sh | New Linux host upgrade script (package upgrades + optional release upgrades with guards). |
| host-setup/linux/setup-github.sh | New Linux script to configure SSH key, git identity, and SSH commit signing with GitHub registration checks. |
| host-setup/linux/install-tools.sh | New Linux tool installer/upgrader for required host tooling, with idempotent behavior and upstream/distro sourcing logic. |
| host-setup/bootstrap.sh | New bootstrap loader that fetches a specific hub ref/commit tarball and hands off to one tooling entrypoint inside the fetched tree. |
| host-setup/agent-safety/test_install.py | New installer self-test suite validating stamp/report behavior and edge cases (corruption, wiring, drift). |
| host-setup/agent-safety/install.py | Adds stamping + --report mode, payload/install digesting, and improved robustness around stamp/registration validation. |
| GOVERNANCE.md | Clarifies that a “loader” is outside hub-tooling rules by scope (single entrypoint) rather than exempt by exception. |
| docs/host-setup.md | Updates the host contract doc to point to host-setup/ for per-platform commands and expands tooling/source rationale. |
| CODESTYLE.md | Adds a Shell section defining when bash is acceptable and sets expectations (pipefail discipline, self-location, shellcheck hygiene). |
| .github/workflows/validate-task.yml | Adds shellcheck gating and expands the self-test step to include bootstrap + host gate + agent-safety installer tests. |
| .gitattributes | Pins LF for the new Python self-tests and documents the updated executed-Python set rationale. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five commits,
1927e9a..56f4d7d. Nineteen files, +4241/-136.What lands
b0d0d13— a shellcheck gate invalidate-task.yml, with the file list fromgit ls-files '*.sh'so a new script is gated without editing the step. Also wiredscripts/test_host_gate.pyinto the self-test step, which was running in no workflow at all.8c6fd27—prose_lint.pychose its rule set from the working directory rather than the scanned repository, so standing in an operational repo and scanning a release repo silently discardedhome-path, the rule that exists because real paths reached a public comment.e2a99f1— a host stamp at~/.claude/agent-safety-stamp.jsonplus--report, so "is this machine current" has an answer that is not a tick in an issue. Also fixedinstall.pytaking no arguments while both wrappers passed"$@", which made--helpperform a full install.6864a9b— the two remainingprose_lint.pyfalse cleans, fixed as a class. An absolute path argument scoped a--diffrun to nothing and exited 0, and an untracked file was invisible to both a diff-scoped run and a whole-tree sweep. Every input to a verdict now derives from the repository being scanned, and every run states the scope it read.56f4d7d— the host bootstrap tooling underhost-setup/linux/, itsbootstrap.shloader,scripts/test_bootstrap.py, and the rules the scripts run under.Review record
Every one of the five closed its Copilot loop on its own pull request. #668 ran five rounds and #667 seven, and between them eighteen findings arrived as suppressed comments rather than as inline threads, thirteen of which were real. Two of those were defects that would otherwise have shipped in the gate this promotion carries: a subtree of new files taking a filesystem walk that applies no ignore rules, and a docstring count that was wrong as well as brittle.
Consequence worth stating
The
GOVERNANCE.md"Hub-Hosted Tooling" paragraph #667 added makes every carrying repository's copy a past revision once this reachesmain. That is the ordinary consequence of a canonical moving rather than a defect, but a repository meeting it first as a red audit line will read it as a surprise. HomeAutomation-Config has already re-vendored it by content rather than by bytes, since a byte copy from a CRLF hub into an LF repository rewrites every line to change one paragraph.Verified on this head
developat56f4d7d, in sync withorigin/develop. Local run of the CI invocations: 223 prose self-tests, the prose gate over 117 files,repo_gate(eol, eol-coverage, sha-pin),spec/validate.pywith 22 cataloged, markdownlint over 45 files, and editorconfig-checker, all clean.Merge as a merge commit, never a squash, and without
--delete-branch: this pull request's head isdevelopitself.