Gate the Host Tool Versions, and Say Where a Tool Must Come From - #638
Conversation
The host contract checked presence, and both host defects this fleet has hit are version facts on a tool that is installed, answers --version, and looks healthy. A distribution gh in the 2.45.x / 2.46.x range is named by the GitHub CLI maintainers as broken by deprecated GitHub APIs, and this host carries 2.46.0 against an upstream 2.97.0. A git-restore-mtime before 2025.08 calls git whatchanged, which current git refuses, so it restores nothing, prints its ordinary statistics and exits 0, and a deploy keyed on mtimes then ships a full copy and reports success. spec/host-tools.json declares the floors as data, and each records the defect it encodes rather than a preference, so most entries carry none deliberately: a floor nobody can justify becomes a host failure nobody can act on. scripts/host_gate.py reads it and replaces the presence-only line in the verification block. A repository layers its own host-tools.json over the hub's, so a repo needing ffmpeg, or needing a tool the fleet calls optional, declares that where it is true. Layering is tighten-only, since lowering a floor from inside the repository a floor protects retires the check, and a rejected relaxation is reported rather than dropped. Two defects came from running it rather than from reading it. A probe that ran and exited non-zero counted as an answer, so a missing git-restore-mtime reported as unreadable, whose remedy is to fix the pattern, rather than absent, whose remedy is to install it. And the install-from line under a floor failure was counted as a second issue. Both are asserted now. OPERATIONS.md records that its two gh limitations were observed on that package and want re-testing against an official install, rather than reading as permanent behavior. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR strengthens the host setup contract by adding a version-aware host tool gate backed by a declarative spec, plus documentation updates explaining required tool sources and measured version floors.
Changes:
- Add
scripts/host_gate.pyto probe required host tools, parse versions via regex, and enforce minimum versions where defects have been measured. - Introduce
spec/host-tools.json(and schema) to declare tools, probes, patterns, optionality, floors, and installation sources; extendspec/validate.pyto shape-check it. - Add a focused unit test suite for the gate and update host/setup and operational docs to reflect the new contract and known
gh/git-restore-mtimehazards.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/validate.py | Adds shape checks for spec/host-tools.json so malformed declarations fail loudly in CI. |
| spec/host-tools.schema.json | Defines the JSON schema for the host tool contract document. |
| spec/host-tools.json | Declares the fleet host tool set, probes, regex patterns, and the two currently measured version floors with sources. |
| scripts/host_gate.py | Implements the host tool probing and version floor enforcement (including local overlay support). |
| scripts/test_host_gate.py | Adds unit tests covering version parsing/comparison, probing behavior, merge tighten-only rules, and shipped declaration invariants. |
| scripts/README.md | Documents the new host_gate.py gate and its rationale and layering model. |
| OPERATIONS.md | Clarifies that observed gh limitations were on distro-packaged gh and points to the new gate/source guidance. |
| docs/host-setup.md | Updates the host verification block to run the new gate and documents required sources/floors for known-bad tool versions. |
| .gitattributes | Pins new Python files to LF line endings. |
Suppressed comments (1)
spec/validate.py:167
- The host-tools.json sort check is case-sensitive (sorted(ht_names)), which is inconsistent with the earlier third-party-tools.json sort check (sorted(..., key=str.lower)). If the intent is “how a reader finds an entry”, sorting should be case-insensitive here as well.
ht_names = [t["name"] for t in host_tools["tools"] if isinstance(t, dict) and isinstance(t.get("name"), str)]
if ht_names != sorted(ht_names):
errors.append("host-tools.json: 'tools' is not sorted by name")
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Two review findings, both accepted. Duplicate detection and sorting for host-tools.json were case-sensitive where the sibling third-party-tools.json checks fold case. The consistency argument is the smaller half. The real one is in the gate: merge keyed on the exact spelling, so a repository writing GH to override the hub's gh silently added a second entry beside it rather than overriding, and the run reported success. Names now fold case in both places, the hub's spelling stands since the gate reports under it, and a case variant cannot relax a floor either. read_tool reported the first declared probe whenever the pattern failed, even where a later probe was the one that ran. That sends a reader to fix a pattern against output they cannot reproduce, and the second probe exists precisely for the platform where the first never runs. The probe that answered is remembered instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 9, 2026
Answering the suppressed comment on
Accepted, fixed in Worth recording that checking whether the inconsistency was actually wrong is what found the defect underneath it. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (4)
spec/validate.py:151
- host-tools.json validation does not currently type-check the optional 'required' field. If a spec entry accidentally sets "required": "false" (string) or similar, host_gate.py will treat it as truthy and fail hosts unexpectedly; since CI does not run JSON-schema validation, validate.py is the only guard that can make this fail loud.
ht_seen.add(name.lower())
if not isinstance(t.get("why"), str) or not t.get("why"):
spec/validate.py:165
- When a tool declares a version floor, validate.py only checks that 'source' is a non-empty object, but it does not verify that the keys/values are usable (linux/macos/windows -> non-empty strings). A typo like "Linux" would silently drop the INSTALL FROM remedy on that platform, undermining the purpose of requiring source for floor entries.
if not isinstance(floor, str) or not re.fullmatch(r"\d+(\.\d+)*", floor):
errors.append(f"host-tools.json: '{name}' minimum {floor!r} must be dot-separated integers or null")
elif not isinstance(t.get("source"), dict) or not t["source"]:
errors.append(f"host-tools.json: '{name}' declares a floor and no 'source', so a host below it is told to upgrade and not where from")
scripts/host_gate.py:218
- host_gate.py assumes the repository-local host-tools.json has a top-level {"tools": [...]} array of objects. If a repo provides a malformed local file (e.g., tools is an object, or entries are not objects), merge() will raise (e.g., AttributeError on entry.get) and the script will crash with a traceback instead of reporting a declaration error (exit 2) as intended.
if not a.no_local and local_path.is_file():
try:
local = json.loads(local_path.read_text(encoding='utf-8'))['tools']
except (OSError, ValueError, KeyError) as e:
print(f'{local_path}: cannot read the repository host tool declaration ({e})', file=sys.stderr)
spec/host-tools.schema.json:2
- The schema URL uses the legacy http://json-schema.org/draft-07/... form. The rest of spec/*.schema.json uses https://json-schema.org/...; updating this avoids mixed/insecure schema fetches in editors and keeps schema headers consistent.
"$schema": "http://json-schema.org/draft-07/schema#",
Four suppressed findings from round 2, all accepted, and three are the same shape: a guard that is present and asserts less than its name. validate.py read `required` for truthiness, so the string "false" is true and would have failed every host on a tool nobody requires. It is type-checked now. CI runs no JSON-schema validation, so this file is the only thing that reads the declaration before the gate trusts it. It also required `source` to be a non-empty object without reading its keys, so a misspelled "Linux" satisfied the check and silently dropped the INSTALL FROM remedy on the platform that needed it. That is the exact failure requiring `source` at all was meant to prevent. Keys are now read against the three the gate looks up, and values must be non-empty. host_gate.py assumed a repository-local file was well formed and would have raised inside merge() on a `tools` object or a non-object entry, reporting a malformed local file as a defect in this script. It is shape-checked before merging and exits 2 as the contract says. The schema header was draft-07 over http where all six sibling schemas are 2020-12 over https, so it moves to 2020-12 and `definitions` becomes `$defs`. Every new validator check was run against a deliberately broken declaration rather than reasoned about, and each one fires. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 9, 2026
Answering the four suppressed comments from round 2. All accepted, fixed in
Correct, including the reason it matters here specifically: nothing runs the schema, so
This is the sharpest of the four, because it is the check defeating its own purpose:
Correct, and the local file is the one input nothing upstream validates. Shape-checked before merging, exit 2 as the contract says, with three cases covering
True and understated. It is not only http against https: six sibling schemas are Each new validator check was run against a deliberately broken declaration rather than reasoned about, since a shape check that never fires is the same defect class as the ones above: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
scripts/host_gate.py:142
- The tighten-only rules for local overrides can be bypassed (and can also crash) when a local host-tools.json uses non-boolean/non-string JSON types. For example, setting
required: 0effectively turns a required tool into optional because0is falsy, but the relaxation guard only rejects the literalfalse. Similarly,minimum: 5(a JSON number) can raise a TypeError in parse_version(). Enforce types forrequired(bool) andminimum(string or null) before applying overrides.
if field == 'required' and merged.get('required', True) and value is False:
rejected.append(f'local tool {name} tried to turn a required tool optional, which is a relaxation, so the hub value stands')
continue
if field == 'minimum':
hub_floor = parse_version(merged['minimum']) if merged['minimum'] else None
scripts/host_gate.py:131
- Local overlay entries are only shape-checked as dicts, but when adding a new tool the code only checks that required keys exist, not that their values have usable types. A local host-tools.json like
{ "name": "ffmpeg", "probes": null, ... }would pass themissingcheck and then crash later (e.g., iteratingtool['probes']), which contradicts the script's stated goal of reporting malformed local files rather than raising.
This issue also appears on line 138 of the same file.
if key not in by_key:
missing = [f for f in REQUIRED_FIELDS if f not in entry]
if missing:
rejected.append(f'local tool {name} adds a new entry without {", ".join(missing)}, so it was ignored')
else:
scripts/host_gate.py:90
- A malformed regex in a declaration (hub or local) will currently raise an uncaught
re.errorin read_tool(), producing a traceback instead of a clear 'declaration is broken' failure. Since local overlays are explicitly not schema-validated, catching regex compile/search errors here keeps the gate in its intended 'report rather than crash' mode.
m = re.search(tool['pattern'], out)
if m:
return 'read', m.group(1), ' '.join(argv)
Round 3, three suppressed findings, one cause: the tighten-only rule and the shape checks read fields for truthiness and presence, and a local file is validated by nothing upstream. The bypass is the one that matters. `required: 0` is falsy and is not False, so a guard written against the literal let it through and a required tool became optional, which is the exact relaxation the rule exists to refuse. Measured against the previous commit rather than argued: pre required:0 -> required=0 rejected=0 <-- BYPASSED pre minimum:5 -> raised TypeError <-- CRASH post required:0 -> required=True rejected=1 post minimum:5 -> handled field_problems now type-checks every field an entry carries, before merge applies any of it, so a wrong type is refused with a reason rather than bypassing a guard or crashing a later read. Only present fields are judged, which is what keeps a one-field override from having to restate an entry. Two crash paths go with it. A new tool declaring `probes: null` satisfied the presence check and failed when the probes were iterated, and a pattern that does not compile raised re.error out of read_tool. The first is a type problem like the rest, and the second also gets a backstop inside read_tool for the case where neither shape check ran. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 9, 2026
Answering the three suppressed comments from round 3. All accepted, fixed in
This is the one that matters, because it is a bypass of the rule the merge layer exists to enforce, not a crash. Measured against the previous commit rather than argued:
Correct, and it is the same defect wearing presence instead of truthiness: the key is there, so the check passes, and the read is what fails.
Correct. Fixed in two places deliberately: The fix is one function rather than three guards. Worth naming the pattern, since this is the third round finding it: every finding on this pull request has been a check that is present and asserts less than its name. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
spec/validate.py:161
- The host-tools.json probe validator claims it requires "non-empty string arrays", but the actual check allows empty strings (it only checks isinstance(a, str)). That can let a declaration pass spec/validate.py while producing probes that cannot execute (and it disagrees with scripts/host_gate.py's stricter probe validation).
probes = t.get("probes")
if not isinstance(probes, list) or not probes or not all(isinstance(p, list) and p and all(isinstance(a, str) for a in p) for p in probes):
errors.append(f"host-tools.json: '{name}' needs 'probes' as a non-empty array of non-empty string arrays")
spec/host-tools.schema.json:70
- host-tools.schema.json allows a tool to declare a version floor (minimum is a string) without requiring a non-empty source object, but spec/validate.py rejects that shape and the documentation treats the install source as part of the floor contract. Encoding this dependency in the schema would prevent a schema-valid but validator-invalid declaration.
"minimum": {
"type": ["string", "null"],
"pattern": "^\\d+(\\.\\d+)*$",
"description": "The lowest acceptable version as dot-separated integers, or null where no floor has been measured. A floor is declared only where a version is known to break a documented procedure."
docs/host-setup.md:220
- The Windows translation note still says to read the interpreter line as
py -3 --version, but the interpreter line in this block is nowpython3 scripts/host_gate.py. As written, the guidance no longer corresponds to the command a Windows host needs to run.
**This block is POSIX, and on native Windows the interpreter line needs translating**, since `python3` is the one name a correctly set-up Windows host does not have. Read it as `py -3 --version` there, matching the contract table above, and run the rest from WSL2 or Git Bash per the shell note. Git Bash inherits the Windows `PATH`, so `python3` reaches the same Store alias stub it does in PowerShell and reports a working interpreter as missing. A PowerShell equivalent of this block is deliberately **not** given here, because it has not been run on a Windows host, and an unverified verification command is worse than none. [#483][issue-483] is where one belongs once someone has executed it.
… note Round 4, three suppressed findings, all accepted. The probes check in validate.py said "non-empty string arrays" and read only the type, so an empty argument passed and produced a probe that cannot execute. It also disagreed with the gate's own check, which read both. The message was right and the code was not, which is the same shape as every other finding on this pull request. The schema let a tool declare a floor with no source, which validate.py rejects, so a declaration could be schema-valid and validator-invalid at once. The dependency is encoded now, and a null minimum is exempt since it declares no floor. Verified with a real validator rather than by reading: the four shapes come out True, False, False, True as intended, and the shipped declaration reports zero errors against the whole schema. The Windows note still read `py -3 --version` after this branch changed that block's command to run the gate, so it pointed at a command the block no longer contains. My own edit made it stale, and it now reads `py -3 scripts/host_gate.py`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 9, 2026
Answering the three suppressed comments from round 4. All accepted, fixed in
Accepted, and it is the fourth round in a row finding the same shape here: the message was right and the code was not. Both halves matter — the claim, and the disagreement with the gate's own
Accepted. Encoded as an That last line is worth having on its own: the shipped file had never been checked against its own schema by anything.
Accepted, and this one is mine: replacing the block's command left the note pointing at a command the block no longer contains. It now reads |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/host_gate.py:268
- The local
host-tools.jsonread has the sameTypeErrorhole as the hub spec read: if the file parses to a non-object (e.g.,[]/null), indexing with['tools']raises and the gate exits with a traceback instead of exit code 2.
try:
local = json.loads(local_path.read_text(encoding='utf-8'))['tools']
except (OSError, ValueError, KeyError) as e:
print(f'{local_path}: cannot read the repository host tool declaration ({e})', file=sys.stderr)
return 2
scripts/host_gate.py:257
main()can crash with a traceback when the hub--specJSON is not an object (e.g.,[]/null). In that casedata['tools']raisesTypeError, which isn't caught, even though the module-level docstring says unreadable declarations should exit 2 with a diagnostic.
This issue also appears on line 264 of the same file.
try:
data = json.loads(Path(a.spec).read_text(encoding='utf-8'))
tools = data['tools']
except (OSError, ValueError, KeyError) as e:
print(f'{a.spec}: cannot read the host tool declaration ({e})', file=sys.stderr)
return 2
Round 5, two suppressed findings, one bug in two places. Indexing the parsed JSON with ['tools'] raises TypeError when the file parses to a list, a null, a string or a number, and TypeError was not in the caught tuple, so the gate produced a traceback where its own docstring promises exit 2 with a diagnostic. Both the hub read and the repository-local read had it. Measured against the previous commit rather than argued: prev hub=[] -> raised TypeError new hub=[] -> exit 2 Seven cases cover both readers across the four non-object shapes plus unparsable text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 9, 2026
Answering the two suppressed comments from round 5. Both accepted, fixed in
Correct on both, and the docstring reference is the right framing: this is the file's own stated contract not being met, rather than a missing nicety. Measured against the commit the finding was raised on: Seven cases now cover both readers across This is round 5, and the run is still producing real defects, every one of them the same family: a guard whose stated scope is wider than what it reads. Worth stating plainly rather than treating the rounds as noise — the gate this PR adds exists to catch exactly that shape on a host, and building it kept reproducing it. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/host_gate.py:257
main()readsdata['tools']from the hub declaration but does not validate that it is a list of objects. Iftoolsis accidentally an object/string/etc, the script will later crash inmerge()/check()with an unhandled exception (stack trace) instead of exiting 2 with a diagnostic as the docstring promises.
try:
data = json.loads(Path(a.spec).read_text(encoding='utf-8'))
tools = data['tools']
except (OSError, ValueError, KeyError, TypeError) as e:
print(f'{a.spec}: cannot read the host tool declaration ({e})', file=sys.stderr)
return 2
scripts/test_host_gate.py:286
- The hub-declaration error paths are tested for top-level JSON/type failures, but there is no regression test for the specific shape that currently causes an unhandled crash:
{"tools": {...}}(i.e.,toolsis not an array). Adding a test here will lock in the intended exit-2 behavior oncemain()validatestools.
class TestMalformedHubFile(unittest.TestCase):
"""The same hole on the hub read, which the docstring says exits 2 with a diagnostic."""
def test_a_top_level_that_is_not_an_object_exits_two(self):
import tempfile
Round 6, two suppressed findings, and the first names an asymmetry I
introduced: the shape guard added last round covered the repository-local
file and not the hub one, so a hub `tools` that is an object, a string, or
an array of non-objects still reached check() and raised.
Fixing only the hub side would leave the same hazard for the next guard, so
both files now come through read_declaration, which returns the entries or
a diagnostic string. Neither file is validated at the point it is read:
spec/validate.py covers the hub one and nothing covers a repository's, so
the reader assumes neither has run.
Measured against the previous commit rather than argued:
prev hub tools={} -> raised TypeError
new hub tools={} -> exit 2
The second finding asked for the regression test that locks it in, which is
here along with a set covering every bad shape through the one reader.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/host_gate.py:202
- merge() allows a repository-local host-tools.json to introduce a non-null
minimum(either via a new tool entry or by adding a floor where the hub had none) without requiring a correspondingsource. In that case, a below-floor failure can print with noINSTALL FROM:line (becausesrcis falsy), which undermines the contract described in spec/host-tools.schema.json / spec/validate.py (declared floors carry a source) and the docs that say the gate “names where to install from”. Consider rejecting any local entry that results in a tool havingminimum != nullbut no usablesource(and validate keys/values like linux/macos/windows non-empty strings) so failures always include a remedy.
missing = [f for f in REQUIRED_FIELDS if f not in entry]
if missing:
rejected.append(f'local tool {name} adds a new entry without {", ".join(missing)}, so it was ignored')
else:
by_key[key] = dict(entry)
Round 8, one suppressed finding. The contract is enforced on the hub declaration by spec/validate.py and by the schema, and neither can see the result of layering, so a repository adding a floor to a hub entry that never carried a source left both files correct on their own and the combination wrong. The symptom is the one requiring a source exists to prevent: a below-floor failure printing no remedy. Measured against the previous commit, with a local file adding a floor to docker, which the hub declares with no source: prev: docker 29.7.1 is below the 99.0 floor - ... (and nothing else) new: docker declares the 99.0 floor with no source, ... (then the failure) contract_problems reads the merged set, and the platform keys with it, since a source naming a key nothing looks up is a source only in shape. Seven cases cover the override, the new local tool, both source defects, the exempt no-floor entry, and the shipped declaration against its own contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 9, 2026
Answering the round-8 suppressed comment, accepted and fixed in
Correct, and it is the sharpest instance of the family this pull request keeps producing: the contract is enforced in two places and neither can see the case that breaks it. The symptom is precisely what requiring a source exists to prevent. Measured against the commit the finding was raised on, with a local file adding a floor to
Eight rounds, and every finding has been one shape: a guard whose stated scope is wider than what it reads. The gate in this pull request exists to catch that shape on a host, and building it reproduced it eight times — which is the most useful thing this review produced, and worth more than any individual fix. |
The maintainer installed gh 2.97.0 from the official repository and git-restore-mtime 2025.08, so both floors this branch declares are now met and the gate passes on this host. That made the two gh limitations testable, and both are gone. OPERATIONS.md carried them as permanent behavior; they were artifacts of the distribution's 2.46.0, which is inside the range the GitHub CLI maintainers name as broken by deprecated APIs. gh pr checks --json -> returned the rollup as JSON gh pr edit --body-file -> applied the change and exited 0 Re-tested rather than inferred from the version, and corrected in both places that carried the claim, since .github/copilot-instructions.md is intent-fidelity and reaches the fleet: the range is named rather than the host, so a reader checks gh --version before concluding the command is unusable. The gh api PATCH form stays documented for a host genuinely stuck on an old gh. The lesson kept in place of the symptoms is that neither was visible to a presence check. A tool old enough to be broken answers --version cleanly, so the defect arrived as two documented workarounds rather than as an upgrade, and the floor is what found it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/host_gate.py:90
- read_tool() only records the first probe that produced output (answered is set once). If multiple probes run but none match the pattern, the resulting
unreadablemessage may cite the wrong command, sending readers to debug output they never need to reproduce. Track the last probe that produced output instead (still avoiding probes that never ran).
answered = None
for argv in tool['probes']:
out = probe(argv)
if out is None:
continue
if answered is None:
answered = ' '.join(argv)
Round 10. read_tool recorded only the first probe that produced output, so where several answered and none matched, the unreadable message cited one arbitrarily and sent the reader to output that need not be the output the pattern has to match. The review proposed tracking the last instead. That trades one arbitrary pick for another: the pattern must match one of the answering probes, so the reader needs all of them, and neither end of the list is more the culprit than the other. Every probe that answered is cited now, and one that never ran is still left out, which was the original point. prev probes cited=1/2 new probes cited=2/2 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 9, 2026
Answering the round-10 suppressed comment. The defect is real and fixed in
The diagnosis is correct and the remedy is not. Tracking the last trades one arbitrary pick for another. Where several probes answered and none matched, the pattern has to match one of them, and nothing makes the final entry more the culprit than the first — the reader needs the whole set to know which output to write a pattern against. Citing the last would produce exactly the finding's own complaint in the mirror case. Every answering probe is cited now, and one that never ran is still excluded, which was the point under the proposal: Worth noting this is the third round in a row touching |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/host_gate.py:99
- read_tool() returns the probe list for an unreadable tool as a string containing backticks ("
and".join(answered)). check() then wraps that string in backticks again ("ran via{how}"), which will produce broken Markdown/code-span formatting and can make the probe citation hard to read. Return a plain separator (no backticks) and let the caller add the code formatting.
if not answered:
return 'absent', None, None
return 'unreadable', None, '` and `'.join(answered)
Round 11. read_tool joined the answering probes with a backtick-bearing separator, and check() then wraps the whole string in backticks of its own, so a multi-probe citation nested one code span inside another. The data layer was deciding presentation, which is the caller's to decide. Plain separator now, and a case asserts the returned string carries no backtick at all, so the next reader cannot reintroduce it without the suite saying so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 9, 2026
Answering the round-11 suppressed comment. Accepted in full, fixed in
Correct, and it is my own regression from the previous round: joining with a separator that carried markup put presentation in the data layer, where the caller had already decided it. A single-probe citation looked fine, so the defect only appeared in the case the previous round added. Plain separator now, and a case asserts the returned string contains no backtick at all, so this cannot be reintroduced without the suite saying so: Three consecutive rounds on |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
spec/validate.py:159
- spec/validate.py currently only checks that host-tools.json
patternis a non-empty string, but scripts/host_gate.py assumes the hub declaration's pattern-compilation errors are caught by spec/validate.py. As-is, an uncompilable regex could ship and only be discovered at gate runtime. Consider compiling the regex here and reporting a clear validator error (matching host_gate.field_problems' behavior).
if not isinstance(t.get("pattern"), str) or not t.get("pattern"):
errors.append(f"host-tools.json: '{name}' needs a non-empty pattern to read a version with")
probes = t.get("probes")
spec/host-tools.schema.json:55
- spec/host-tools.schema.json allows empty-string probe arguments (probes[].items has only type: string), but spec/validate.py and scripts/host_gate.py both treat empty arguments as invalid. This can make a declaration schema-valid but still rejected by the validator/gate. Tighten the schema to require non-empty strings for probe arguments.
"type": "array",
"minItems": 1,
"items": { "type": "string" }
}
Round 12, two suppressed findings, and one cause: a declaration is read by the schema, by spec/validate.py, and by the gate, and the three disagreed about two rules, so a file could be valid to one and rejected by the next. validate.py type-checked the pattern without compiling it, while host_gate.py names validate.py as what covers the hub declaration, so an uncompilable hub regex would have shipped and surfaced at gate runtime, which is the reader least able to fix it. prev: Spec validation OK new: 'gh' pattern does not compile (missing ), unterminated subpattern ...) The schema allowed an empty probe argument where validate.py and the gate both reject one, so a declaration could be schema-valid and rejected by both readers behind it. Verified with a real validator, since nothing in CI executes the schema: the four probe shapes come out True, False, False, False, and the shipped declaration reports zero errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 9, 2026
Answering the two suppressed comments from round 12. Both accepted, fixed in
Correct, and it names the coupling exactly:
Correct. Both A/Bs were run by the corrected method described in the earlier correction comment: the prior |
Uh oh!
There was an error while loading. Please reload this page.
…ate (#635) Promotion of develop at 2082547, six squashes since the last one. Closes#633. #631 routes the README by reader and documents the GH_WRITE_GUARD_ALLOW grant where a denied cross-owner write puts the reader. #632 moves readme.sections from intent to letter with four checks beside it, backed by spec/readme-sections.json and spec/third-party-tools.json, and settles the tagline rule. #634 adds repo_gate.py --check eol-coverage, reading the line-ending pins against the tree rather than only against .editorconfig. #636 and #637 repair two readers Copilot found on this pull request, both defects develop already carried: a tool row required both outer table pipes that GitHub's Markdown makes optional, and a retired badge written as an inline image was invisible to a scan that read reference definitions alone. #638 turns the host contract's presence check into a version gate, and retires two gh workarounds that were artifacts of a stale distribution package, re-tested on an upgraded host rather than inferred. Four carried files owe a downstream re-vendor and none is recorded in the TODO.md entry yet. GOVERNANCE.md Repository Details is verbatim, so the audit reports it, and it propagates a rule: the About description is the tagline alone, and Docker Hub receives it from the About panel rather than from the README. CODESTYLE.md item 4 and .gitattributes are intent, so nothing reports them, and the second couples to the new gate through the forward-declared mark. .github/copilot-instructions.md is intent and propagates a correction rather than a refresh, so a repo left on the old copy is wrong rather than merely stale.
The host contract checked presence, and both host defects this fleet has actually hit are version facts on a tool that is installed, answers
--version, and looks healthy. Neither is visible to the check that was there.The two defects, measured
A distribution
gh. The GitHub CLI maintainers state that the community-distributed2.45.x/2.46.xis "broken due to deprecated GitHub APIs". This host carries Debian2.46.0-3against an upstreamv2.97.0, and the binary embeds the deprecated field:Both
ghlimitations recorded inOPERATIONS.mdare that deprecation class, and both were observed on that package.An old
git-restore-mtime. Debian and Ubuntu package 2022.12, which callsgit whatchanged; git2.51announced that command's deprecation and current git gates it behind a hidden flag a caller cannot pass through. The tool then restores nothing, prints its ordinary statistics, and exits 0, so a deploy keyed on mtimes ships a full copy and reports success. Confirmed against the real releases:v2022.12callswhatchangedat line 321,v2025.08does not.Two things worth recording because they are counter-intuitive. A newer git is the trigger rather than the remedy, so this host (git 2.47.3, which still allows
whatchanged) runs the stale tool correctly and cannot reproduce the failure. And neither release has any shallow-clone awareness — a shallow clone silently produces wrong mtimes with the same success-shaped report — which is a separate latent hazard, not this one.What ships
spec/host-tools.jsondeclares the floors as data, and each one records the defect it encodes rather than a preference. Only two floors exist, and everything else is presence-only, deliberately: a floor nobody can justify becomes a host failure nobody can act on. A test asserts the floor set rather than a count, so adding one without a reason fails.scripts/host_gate.pyreads it and replaces the presence-only line in the verification block. Three states are kept apart because their remedies differ: absent means install it, unreadable means the declared pattern is wrong and the fix is in this repo, read means the floor applies.Extensible, per the request
A repository adds its own
host-tools.jsonat its root and the gate layers it over the hub's. The version-extraction problem is handled by the data carrying both halves already —probessays how to ask,patternsays how to read the answer — so a new tool is a new entry rather than new code.Layering is tighten-only: add a tool, raise a floor, or turn an optional tool required; never lower a floor or turn a required tool optional, since those retire a fleet check from inside the repository it protects. A rejected relaxation is reported rather than dropped. Run against a fixture doing all three:
Two defects found by running it, not reading it
git restore-mtime --versionon a host without the tool prints git's own error and exits 1, so the tool reportedunreadable(fix the pattern) instead ofabsent(install it) — opposite remedies. The exit code now decides.INSTALL FROMline was appended as its own issue, so one finding printed2 issue(s). It now rides on the finding.Both have tests. They are in the suite because the first live run produced them, which is the argument for running a gate against a real host before shipping it.
Scope, stated rather than assumed
git-restore-mtimeis needed by no procedure in this repo — it is one repository's deploy path, already fixed there in ptr727/Blog#75. It is declared optional so its floor and reason are recorded without telling 21 repos to install a tool they do not use.docs/host-setup.mdnames an installer for the first time, which its own contract section says it deliberately avoids. That is called out in the text rather than done quietly: here the source is the requirement, not a convenience, because the distribution package is the failing case. Windowswingetand macOS Homebrew track upstream, so neither raises the hazard.OPERATIONS.mdnow records that its twoghlimitations were observed on that package and want re-testing against an official install, rather than reading as permanent behavior. This does not claim the upgrade fixes them — currentghtrunk's edit path no longer grep-matchesprojectCards, but the struct field survives, and nothing here has run a currentgh.The premise, since verified on this host
The maintainer installed
gh2.97.0 from the official repository andgit-restore-mtime2025.08 while this was in review, so both floors are now met and the gate passes:That made the two
ghlimitations testable rather than suspected, and both are gone:The second was tested by making this description's own update with it, so the command under test did work that was wanted rather than firing as a probe.
So
OPERATIONS.mdno longer hedges: two workarounds it carried as permanent behavior were artifacts of the distribution's 2.46.0..github/copilot-instructions.mdis corrected with them, and since that file isintentfidelity and reaches the fleet, it now names the version range rather than this host, so a reader checksgh --versionbefore concluding the command is unusable. Thegh api PATCHform stays documented for a host genuinely stuck on an oldgh.This is the argument for the whole change, made by accident. Neither symptom was visible to a presence check — a tool old enough to be broken answers
--versioncleanly — so the defect arrived as two documented workarounds instead of an upgrade, and survived months of use that way. The floor is what found it.Verification
scripts/test_host_gate.py— 27 tests, covering version padding (2025.08against2025.8.0), the three read states, every merge rule in both directions, and the shipped declaration itself.spec/validate.pyshape-checks the new file, including that a declared floor carries asource, so a host below it is told where to install from rather than only to upgrade.prose_lint.py --diff develop,repo_gate.py --check eol --check eol-coverage,spec/audit.py --selftest, editorconfig-checker, markdownlint-cli2 and cspell all clean. New.pyfiles are pinned LF in.gitattributesand the new.jsonfiles are CRLF, matching the repo default.🤖 Generated with Claude Code