Skip to content

Carry the Host Bootstrap Tooling, and the Rules It Runs Under - #667

Merged
ptr727 merged 7 commits into
developfrom
host-bootstrap-tooling
Aug 11, 2026
Merged

Carry the Host Bootstrap Tooling, and the Rules It Runs Under#667
ptr727 merged 7 commits into
developfrom
host-bootstrap-tooling

Conversation

@ptr727

@ptr727 ptr727 commented Aug 11, 2026

Copy link
Copy Markdown
Owner

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

Copilot AI lite review requested due to automatic review settings August 11, 2026 02:21

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

Adds first-class host bootstrap tooling to make the existing host contract actionable on fresh Linux installs, and wires in a regression test that keeps the bootstrap invariants and spec/tooling coverage from drifting.

Changes:

  • Add host-setup/ bootstrap + Linux host tooling scripts for installing/upgrading required tools and configuring GitHub signing.
  • Document the new host setup workflow and clarify governance boundaries for loader vs hub-hosted tools.
  • Add a new self-test (scripts/test_bootstrap.py) and run it in CI; pin LF line endings for the new test.

Reviewed changes

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

Show a summary per file
File Description
scripts/test_bootstrap.py New test enforcing bootstrap loader invariants and spec/tooling coverage.
host-setup/README.md New usage doc for bootstrapping and running host setup tooling.
host-setup/linux/upgrade-host.sh New host package/release upgrade script for Debian/Ubuntu-based systems.
host-setup/linux/setup-github.sh New script to configure SSH key, git identity, and SSH-based commit signing.
host-setup/linux/install-tools.sh New script to install/upgrade required host tools from distro/upstream sources.
host-setup/bootstrap.sh New bootstrap loader that fetches a ref tarball and hands off to one entrypoint.
GOVERNANCE.md Clarifies loader vs tool boundary under hub-hosted tooling rules.
docs/host-setup.md Updates contract doc to point at host-setup/ and explains sourcing decisions.
CODESTYLE.md Adds Shell guidance for bootstraps/host tools and their constraints.
.github/workflows/validate-task.yml Runs the new bootstrap self-test in the self-test step.
.gitattributes Pins LF line endings for the new bootstrap test.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread host-setup/README.md Outdated
Comment thread scripts/test_bootstrap.py
Comment thread CODESTYLE.md Outdated
Comment thread scripts/test_bootstrap.py Outdated
Copilot AI review requested due to automatic review settings August 11, 2026 02:35

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)

CODESTYLE.md:495

  • CODESTYLE.md now states "set -Eeuo pipefail" must be the first line after the shebang, but the repository's own scripts (including the referenced repo-config/configure.sh and the new host-setup/*.sh scripts) place comments between the shebang and the set command. This makes the rule text incorrect and likely to cause churn for contributors trying to follow it.
- **`set -Eeuo pipefail`, first line after the shebang.** Without `-e` a failed command in the middle of a sequence lets the rest run against a state nobody checked, and without `pipefail` a pipeline reports the exit of its last stage, so a fetch that failed reads as an answer when a parser downstream succeeds on an empty input. `-E` carries an `ERR` trap into functions and command substitutions, so a script that later adds one is not surprised by where it does not fire.

@ptr727

ptr727 commented Aug 11, 2026

Copy link
Copy Markdown
Owner Author

Correct, and it contradicted the file the same bullet cites as its example. Fixed in the commit above.

Measured across every tracked script rather than the two the finding names:

  catalog/snippets/devcontainer/dotnet/post-create.sh   set on line 2
  catalog/snippets/devcontainer/python/post-create.sh   set on line 2
  host-setup/agent-safety/install.sh                    set on line 7
  host-setup/bootstrap.sh                               set on line 10
  host-setup/linux/setup-github.sh                      set on line 10
  host-setup/linux/install-tools.sh                     set on line 11
  host-setup/linux/upgrade-host.sh                      set on line 11
  repo-config/configure.sh                              set on line 30

Six of eight carry a header comment first, and the outlier at line 30 is repo-config/configure.sh, which that same bullet names as the worked example for its neighbouring rule. A contributor following the text as written would have moved header blocks in six files to satisfy a rule none of them broke.

The rule now reads "before the first command the script runs", and says a header comment sits above it as this repository's scripts do.

That is what the rule was always protecting. -e, pipefail and -E guard what executes, so the property is that nothing executes before they are set, which is a statement about order. "First line after the shebang" was a proxy for that order which happens to be false for a comment, and a proxy is only safe while nothing else can occupy the space it assumes is empty.

Copilot AI review requested due to automatic review settings August 11, 2026 02:47

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 (2)

host-setup/bootstrap.sh:123

  • If tar -xzf fails in download_tree(), the script exits before rm -f "$archive" and before TREE is set, so the cleanup trap will not remove the downloaded tarball (or the partially created $DIR/tree directory). Over time, repeated failures can leave large artifacts in the cache directory.
    # The archive holds one top-level directory named for the repository and the revision.
    # Extracting into a directory of our own keeps a second run from reading the first one's tree.
    rm -rf "$DIR/tree"
    mkdir -p "$DIR/tree"
    tar -xzf "$archive" -C "$DIR/tree" --strip-components=1 ||
        die "Could not extract the downloaded archive"
    rm -f "$archive"

host-setup/linux/setup-github.sh:571

  • setup-github.sh --configure currently treats SSH authentication as the only gate, but the header comment says registering the managed key as an authentication key is also a gate. As written, the run can complete when authentication succeeds via some other key (or when the managed key is not registered), and --dry-run will also fail on a fresh host because it still attempts the SSH/auth checks it would have just performed.

Consider (1) explicitly checking github_auth_key_registered and stopping with registration_needed when it is not registered, and (2) skipping the SSH/registration probes under --dry-run (similar to how the signing check is already skipped).

    step "Checking SSH authentication to GitHub"
    resolve_github_user
    if [[ -z $GITHUB_USER ]]; then
        registration_needed "authentication" "Authentication"
        die "SSH authentication to GitHub failed"

Copilot AI review requested due to automatic review settings August 11, 2026 02:58
@ptr727

ptr727 commented Aug 11, 2026

Copy link
Copy Markdown
Owner Author

Both suppressed comments on 440b9fd are correct, and both are fixed in 525449d. Each was reproduced before the fix and after it, rather than reasoned about.

The cleanup trap

cleanup removed $TREE, which download_tree sets only after extraction succeeds. A failed extract therefore left both the tarball and the part-written tree behind, and the trap that exists to prevent exactly that returned early because the variable naming what to remove had never been assigned.

Reproduced with a shimmed tar that exits non-zero:

CACHE/tree
CACHE/tree.tar.gz
total: 592K

Once per failed attempt, so a host retrying a bad ref accumulates a copy each time.

The two paths this script creates under DIR are now named by tree_path and archive_path, which the download and the trap share, and the trap removes those rather than the one that means the run finished. DIR itself is still never removed, since --dir may name a directory the caller owns.

Verified after: a failed extract leaves 0 paths, a successful run without --keep leaves 0 paths, and --keep still keeps the tree while the archive is still removed on success.

The authentication gate

The stronger half of the finding is right and I had it wrong in the more interesting direction. --configure treated any successful authentication as the gate, so a host whose managed key exists nowhere, but which reaches GitHub with some other key, ran to Done and exit 0 while noting the fact in an ordinary info line.

That is the exact state this script exists to leave behind, reported as success. The managed key is what makes a host revocable on its own, so a fleet host that authenticates with a shared or forwarded key looks configured and is not: revoking it alone would not cut its access.

It now prints the registration block and warns. It still does not fail, because the host genuinely does reach GitHub, which is the same treatment the signing registration already gets and the reason the header calling both "gates" was overstated for one of them.

The dry-run half, which is a different defect than described

--dry-run does not fail because it "attempts the SSH checks". It fails because registration_needed read the public key with cat, and a dry run does not create one:

  3. Paste the key below, and give it this host's name

cat: .../.ssh/id_ed25519.pub: No such file or directory

A first-time reader's safest possible command printed the browser instructions and then died at step 3, which is the line they came for. It now says no key exists yet and that a run which is not a dry run creates it.

The "Then run this again. Nothing after this point works until the key is registered." line moved out of registration_needed to its callers, because it was true for the authentication failure, false for signing, and false for a host authenticating with another key. One shared consequence for three different situations was how the third one came to be described as an ordinary line.

A note on the instrument

My first attempt to simulate a fresh host set HOME to a sandbox. That does not work: ssh expands ~ from the passwd entry rather than $HOME, so the probes kept using this host's real key and the run reported success for the wrong reason. ssh -G showed it, and the reproduction above uses a shimmed ssh instead.

Recording it because the sandbox still demonstrated something real, just not what it was built to test: the managed key was absent while another key authenticated, which is the first finding, arrived at by accident while measuring the second one wrongly.

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 (2)

host-setup/linux/setup-github.sh:284

  • github_signing_key_registered() treats an empty signing-key list as an error: when the API returns "[]" (no signing keys), grep finds no "key" fields, exits 1, and the function returns 2 ("could not be read") instead of 1 ("not registered"). That misreports an unregistered signing key as an unknown network failure and can skip the expected browser-registration prompt.
    local body payload keys
    body=$(key_body) || return 2
    payload=$(fetch "https://api.github.com/users/$GITHUB_USER/ssh_signing_keys") || return 2
    keys=$(grep -oE '"key": *"[^"]*"' <<< "$payload" | sed 's/"key": *"//; s/"$//') || return 2
    grep -qxF "$body" <<< "$keys"

scripts/test_bootstrap.py:24

  • Unused import: sys is imported but never referenced in this self-test script.
import json
import re
import sys
from pathlib import Path

Copilot AI review requested due to automatic review settings August 11, 2026 03:23
@ptr727

ptr727 commented Aug 11, 2026

Copy link
Copy Markdown
Owner Author

Both correct, both fixed in 8a5f12a. The signing one is the better finding of the two and it fails on the exact case it exists for.

An empty signing-key list read as a network failure

keys=$(grep -oE '"key": *"[^"]*"' <<< "$payload" | sed '...') || return 2

An account with no signing key returns [], so grep matches nothing and exits 1. Under pipefail that becomes the pipeline's status, and the || return 2 maps it to "could not be read".

The caller warns on a 2 rather than prompting, so the account was told GitHub might be unreachable instead of being shown its key and the page to paste it into. That account is precisely the one the prompt exists for, which is why the check was wrong on its primary case and right on every host already configured, including this one. Running it here would never have shown it.

Reproduced under the script's own shell options, which is the part that matters:

  []                        rc=2  COULD NOT BE READ
  [{"key": "otherkey"}]     rc=1  not registered
  [{"key": "mykey"}]        rc=0  registered

A first attempt to reproduce it in a plain shell showed rc=0 for all three and looked like a refutation. Without pipefail the pipeline reports sed's status, and sed succeeds on empty input, so the harness has to set the same options the script sets or it measures a different program.

grep exiting 1 is now read as an empty list and only a higher status is a failure to read. All four outcomes verified, including a failing fetch still returning 2.

github_auth_key_registered was checked for the same shape and does not have it: .keys returns plain text, and its grep -qxF is the final command, whose 1 correctly means not registered.

Worth naming

This is the pipefail with an early-exiting reader trap, which CODESTYLE.md states two bullets above the one this pull request edits, in a script this pull request adds:

Under pipefail, a producer writing to a closed pipe exits non-zero, so
curl ... | grep -q reports a successful fetch as a failure whenever the
match is found early enough.

Same mechanism, opposite end of the pipe: there the producer's status is lost, here the consumer's "no match" is indistinguishable from a failure. The rule as written names the case that bit us before, and the general form is that a pipeline under pipefail has one status for several distinguishable outcomes, so any pipeline whose stages disagree about what non-zero means has to be split before it is tested.

The unused import

Correct, sys was never referenced. Removed. The exit path uses raise SystemExit(main()), so it still exits non-zero on a seeded failure, which I confirmed rather than assumed.

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)

docs/host-setup.md:261

  • In Markdown docs, reference definitions within each group must be alphabetized by reference name (GOVERNANCE.md "Documentation Style Conventions" -> Markdown). In this <!-- Repo --> block, scripts-dir/spec-dir appear before host-tools and before issue-483/operations, so the group is not sorted and will drift from the documented convention.
[host-gate]: ../scripts/host_gate.py
[host-setup-dir]: ../host-setup/
[scripts-dir]: ../scripts/
[spec-dir]: ../spec/
[host-tools]: ../spec/host-tools.json

ptr727 and others added 6 commits August 10, 2026 20:30
A machine with a fresh operating system had no path to the state this repository's contract describes. docs/host-setup.md said which tools a host must provide and deliberately named no installer, promising that per-platform install commands were tracked separately. Nothing tracked them. This is that half of the promise.

host-setup/linux holds three tools, each proven on Debian 12 and 13, Ubuntu 24.04 and 26.04, Proxmox, and both WSL distributions. 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 the packages of the current release from the release itself, and refuses a release move on Proxmox and on any distribution it does not know. 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.

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 ref is first class rather than an escape hatch, because a repository testing a hub change before it promotes cannot be served by a loader that only reaches main.

The three scripts 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. About thirty lines each are duplicated, identically, and the README says so.

GOVERNANCE gains a scope paragraph rather than an exception. The rule that reaching the hub is a checkout governs a tool that reads hub content, and a loader reads none, so it sits outside that rule rather than being excused from it. The bound is content rather than caller, and it is testable: a loader references no path inside the tree it fetches except the single entry point it hands control to. scripts/test_bootstrap.py asserts it, and asserts that every tool the spec requires on Linux is one the tooling can install, which is the only connection between the floors and the installer. Nothing joins them at runtime, deliberately: the gate measures a host and the tooling changes one.

CODESTYLE gains a Shell section, since a repository carrying this much bash should say what breaks without the rules it follows.

Verified by running rather than by review. Every gate this repository runs passes over the branch. The three tools report 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.

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

The coverage assertion skipped any required tool whose declaration named no
source.linux, on the reading that such a tool is not a Linux concern. That is
wrong: source.linux is prose saying where a tool comes from, not a marker of
which platform it applies to. It skipped docker, git and uv, which is half the
required set, and docker being skipped meant NOT_MANAGED was never exercised
at all. So the check that exists to prove every required tool is installable
was proving it for three of six, and the recorded exception it exists to
enforce was dead. Scope is now the spec requiring the tool, and all six
resolve: docker to its recorded exception, the other five to managed.

spec_tools raised on a malformed or unreadable declaration, ending the run
with a traceback and taking the checks after it. It now records the failure
beside the others and returns nothing, which is what every other check in the
file does.

Both link findings are correct. GOVERNANCE names four files that keep inline
links and neither of these is among them, so the three bullets in
host-setup/README.md and the one in CODESTYLE.md become reference-style, with
the definitions merged into the existing groups and sorted by reference name
alone.

Each tightened property was watched to fail before it was trusted: adding
docker to the managed list, dropping uv from it, and corrupting the spec each
produce one named finding and exit 1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rule said set -Eeuo pipefail is the first line after the shebang. Six of
this repository's eight scripts put a header comment there first, including
repo-config/configure.sh, which the same bullet cites as its worked example
and which reaches set on line 30. So the rule was contradicted by the file it
pointed at, and a contributor following it would have moved comment blocks
for nothing.

What the rule protects is that nothing executes before the guards are set,
which is a statement about order rather than about line numbers. It now says
that, and says a header comment above it is what the repository does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings, all reproduced before being fixed and after.

The bootstrap trap removed TREE, which is set only once extraction has
succeeded, so a failed extract left both the tarball and a part-written tree
in the cache. Shimming tar to fail left 592K behind on every attempt, and a
host retrying a bad ref accumulates one copy per try. The two paths this
script creates are now named by helpers that the download and the trap share,
and the trap removes those rather than the one that means the run finished.
DIR itself is never removed, since --dir may name a directory the caller owns.

setup-github.sh --configure treated any successful authentication as the
gate. A host whose managed key exists nowhere, but which reaches GitHub with
some other key, ran to "Done" and exit 0 while printing one ordinary line
about it. That is the state the script exists to leave behind, reported as
success. It now prints the registration block and warns that the managed key
is registered nowhere, so revoking this host alone would not cut its access.
It still does not fail, because the host does work, which is the same
treatment the signing registration already gets.

registration_needed read the public key with cat. A dry run does not create
one, so a first-time reader's safest possible command printed the browser
instructions and then died on a cat error at step 3, which is the line they
came for. It now says no key exists yet and that a real run creates it.

The "nothing after this point works" line moves out of registration_needed to
its callers, because it was true for the authentication failure, false for
signing, and false for a host authenticating with another key.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github_signing_key_registered piped the API response through grep into sed
and mapped any non-zero to "could not be read". An account with no signing
key returns an empty list, so grep matched nothing and exited 1, pipefail
made that the pipeline's status, and a definite no was reported as a possible
network failure.

The caller warns rather than prompting on a 2, so an account with no signing
key was told GitHub might be unreachable instead of being shown the key and
the page to paste it into. That account is exactly the one the prompt exists
for, so the check failed on its primary case and passed on every host that
was already configured, which is why running it here never showed it.

Grep exiting 1 is now read as an empty list and only a higher status is a
failure to read. Verified for all four outcomes: an empty list and a list
without this key both report unregistered, a list with it reports registered,
and a failing fetch still reports unreadable.

This is the pipefail-with-an-early-exiting-reader trap that CODESTYLE.md
names two bullets above the one this pull request edits, in a script this
pull request adds.

Also drops an unused sys import from scripts/test_bootstrap.py.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
scripts-dir and spec-dir landed between host-setup-dir and host-tools, where
the four definitions this branch adds went in beside the line that first used
them rather than in name order. They now sit after operations.

The sort key is the reference name alone, so a sort over the whole definition
line would not have caught it and would invert every prefix pair it touched.
Checked by keying every reference group in every Markdown file this branch
touches against its own sorted order: this was the only one out of order, and
it is now the only kind of check that can say so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 11, 2026 03:31
@ptr727
ptr727 force-pushed the host-bootstrap-tooling branch from 8a5f12a to f39892a Compare August 11, 2026 03:31
@ptr727

ptr727 commented Aug 11, 2026

Copy link
Copy Markdown
Owner Author

Correct, and fixed in f39892a. The branch is also rebased onto develop at 6864a9b, which advanced with #668 while this was in review.

[scripts-dir] and [spec-dir] sat between [host-setup-dir] and [host-tools]. All four of the definitions this branch adds went in beside the line that first used them rather than in name order, and two of the four happened to land correctly, which is why it read as sorted at a glance.

They now sit after [operations]:

agent-safety, audit, bootstrap, devcontainer, governance-git-and-commit-rules,
host-gate, host-setup-dir, host-tools, issue-483, operations, scripts-dir,
spec-dir, ssh-signing, standup, write-guard

Checked over the population rather than the instance

A finding names one group and says nothing about how many there are, so I keyed every reference group in every Markdown file this branch touches against its own sorted order:

  CODESTYLE.md          <!-- Repo -->        ok (13 refs)
  CODESTYLE.md          <!-- External -->    ok (9 refs)
  TODO.md               <!-- Issues -->      ok (24 refs)
  TODO.md               <!-- Repo -->        ok (34 refs)
  docs/host-setup.md    <!-- Repo -->        OUT OF ORDER at [scripts-dir]
  docs/host-setup.md    <!-- External -->    ok (5 refs)
  host-setup/README.md  <!-- Repo -->        ok (7 refs)
  scripts/README.md     <!-- Internal -->    ok (13 refs)

One group, one fix, and zero out of order afterwards. Worth doing because the alternative is fixing the reported instance and leaving an unknown number of siblings, which reads identically from the outside.

The check has to extract the reference name and compare on that alone. GOVERNANCE.md "Documentation Style Conventions" says so, and the reason it matters here is mechanical: - precedes ] in byte order, so sorting whole definition lines inverts every prefix pair, and a sort -c over the block passes on the inverted order. A checker built the obvious way would have reported this block clean and quietly damaged [repo-config] against [repo-config-settings] in any block it was used to fix.

Every gate re-run after the rebase: the self-test, prose undiffed over the whole tree, shellcheck over all tracked scripts, editorconfig, repo gates, spec validation, and markdownlint over every Markdown file.

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.

Suppressed comments (2)

host-setup/README.md:16

  • The quick-start claims "a host that has nothing", but bootstrap.sh requires tar (and will exit if missing). The apt install line should include tar so the 3-line path works on a minimal base image.
sudo apt-get update && sudo apt-get install -y curl ca-certificates

host-setup/linux/install-tools.sh:732

  • The comment says failures are collected so one unreachable upstream doesn't strand the run, but many paths call die, which exits the script immediately (for example install_keyring and several *_install functions). Either make tool installs return non-zero instead of exiting, or adjust this comment so operators aren't misled about failure behavior.
# Install or upgrade one tool.
# A failure is collected rather than fatal, so one unreachable upstream does not strand the rest of the run.
apply_tool() {

Comment thread host-setup/bootstrap.sh
--dir is caller supplied, so "tree" under it is not necessarily ours. The
loader removed that path twice, once before extracting and again from the
exit trap, on the strength of its name. Pointing --dir at a directory that
already held a tree destroyed it, and the run is often root.

A tree now carries a marker this loader writes when it creates one, and a
tree without that marker is refused rather than removed, at both sites. --dir
must also be an absolute path and may not be the root, since everything below
it is created and removed.

Proven on the case that matters rather than the absurd one: a directory
holding a tree with a file in it is refused and the file survives. --dir /
and a relative path are refused by argument parsing. A normal run still
leaves nothing behind, --keep still keeps the tree, and a second run reuses
its own tree rather than refusing it.

The exit trap no longer re-reports the refusal, which printed the same error
twice after the message that actually stopped the run.

The three line snippet installs tar, which the loader requires and checks
for. Both current base images carry it, so this is the "a host that has
nothing" claim being made true rather than a fix to a failure seen.

The comment above apply_tool claimed every failure is collected. Several
install paths end the run instead, and two kinds are mixed there: a refusal
is deliberate and stays fatal, while an upstream lookup that cannot be
answered is the case collecting exists for and ends the run today. The
comment now states the split, and TODO.md carries the behavior change, which
wants its own verification per distribution rather than riding along here.

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

ptr727 commented Aug 11, 2026

Copy link
Copy Markdown
Owner Author

Both suppressed comments on f39892a are answered here, both fixed in 768fd55.

tar in the three-line snippet

Accepted, with a measurement that narrows the claim. bootstrap.sh does require tar and does check for it, so the failure was diagnosed rather than cryptic: the loader runs command -v tar and dies with "tar is required to unpack the tooling".

And both images this targets already carry it:

  debian:stable-slim   HAS tar
  ubuntu:24.04         HAS tar

So this is not a fix to a failure anyone would have hit on a supported base. It is the "a host that has nothing" claim in that heading being made true by construction rather than by the current contents of two images, which is worth one word in an apt line. Added.

The apply_tool comment

Correct, and the comment was the wrong half to trust. apply_tool does collect a non-zero return from a tool's install function, and several install paths call die and end the run before that can happen.

Two kinds are mixed there, and only one of them is a defect:

  • A refusal, which should stay fatal. install_keyring on a key that does not sign the repository it claims to, github_latest_tag on a checksum mismatch, and a declined confirm. Each means nobody vouched for what would be installed, and collecting one to carry on installing the rest is worse than stopping.
  • An upstream lookup that cannot be answered, which is exactly what collecting exists for. node_install failing to read the current long term support line, uv_install finding no build for the architecture, and dotnet_install finding no package for the distribution. Each of those is one unreachable upstream stranding every tool after it, which is the sentence the comment made and the behavior contradicts.

The comment now states the split rather than claiming the collecting behavior for all of them.

The behavior change is recorded in TODO.md rather than made here. Turning those three into a collected failure changes the install path on every supported distribution, so it wants verification per distribution rather than riding along with a migration whose whole claim is that these scripts already behave as verified.

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 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (3)

host-setup/bootstrap.sh:113

  • remove_tree checks [[ -e $tree ]] without quoting. If --dir contains spaces, $tree will be split into multiple words and the test will error, potentially skipping the intended safety guard. Quote $tree in the existence test.
    tree=$(tree_path)
    [[ -e $tree ]] || return 0
    tree_is_ours || die "$tree exists and this loader did not create it, so it will not be removed. Choose another --dir."
    rm -rf "$tree"

host-setup/bootstrap.sh:147

  • In cleanup, [[ -e $(tree_path) ]] is unquoted. With a --dir that contains spaces, this condition can fail due to word-splitting and cause incorrect cleanup behavior. Quote the tree_path result.
    if [[ -e $(tree_path) ]] && ! tree_is_ours; then
        rm -f "$(archive_path)"
        return 0

host-setup/bootstrap.sh:105

  • tree_is_ours uses an unquoted command substitution ([[ -e $(marker_path) ]]). If --dir contains spaces, word-splitting will break the test (and can make the loader mis-handle ownership checks). Quote the path produced by marker_path to make --dir robust for valid absolute paths with spaces.

This issue also appears in the following locations of the same file:

  • line 110
  • line 145
tree_is_ours() { [[ -e $(marker_path) ]]; }

@ptr727

ptr727 commented Aug 11, 2026

Copy link
Copy Markdown
Owner Author

All three suppressed comments on 768fd55 are disproven, and no change is made for them. They share one premise, and the premise is false.

Each says an unquoted expansion inside [[ ... ]] word-splits when --dir contains a space, at remove_tree line 113, at cleanup line 147, and at tree_is_ours line 105 with 110 and 145 named as siblings.

Bash does not word-split inside [[ ]]. It is a compound command, and word splitting and pathname expansion are not performed on unquoted expansions within it. That is the difference between [[ and [.

Measured, on this host

GNU bash 5.2.37, against a directory literally named sp ace:

  [[ -e $t ]]        true, found it
  [[ -e $(mp) ]]     true, found it
  [ -e $t ]          [: /tmp/.../sp: binary operator expected

The third line is the one that makes the point rather than the first two. Single brackets do split and produce exactly the error the findings describe, double brackets do not, and every construct the three findings name is a double bracket.

Measured again, on the program rather than the construct

The bracket behaviour is the mechanism; what matters is whether the loader works and whether the guard the findings say would be skipped still fires. Both, with a space in the path throughout:

==> Fetching ptr727/ProjectTemplate at host-bootstrap-tooling
  Commit: 768fd554489a773b97a14a2e504d385566389e8d
  Extracted to /tmp/.../sp ace/run/tree
  left behind: 0 path(s)

ERROR: /tmp/.../sp ace/mine/tree exists and this loader did not create it, so it
       will not be removed. Choose another --dir.
  user file: survived

A full run fetches, extracts, reports and cleans up under a directory with a space, and the ownership guard refuses correctly there too. The specific failure the findings predict, the guard being skipped, does not occur.

Why this is a decline and not a quiet fix

Adding the quotes would be harmless, and some would call it better style. Doing it and calling it a fix would record a false statement about bash in a commit message and in this conversation, where the next reader takes it as fact and carries it into code that uses single brackets, where it is true and matters.

So the verdict is disproven rather than fixed. If the quotes are wanted for consistency they are a style change with a style reason, and they are not this.

This is the first round on this pull request where the suppressed channel produced findings that were wrong. The eight earlier ones were all correct and all fixed, three of them defects that would have shipped. That is the rule working in both directions rather than an argument against reading them: each is judged against the code, never against the channel it arrived in or the confidence label attached to it.

Earlier findings, paired

earlier=8 in the digest is eight findings across five rounds, answered by five comments, one per round: 82ca456 the shell rule wording, 440b9fd the cleanup trap and the authentication gate, 525449d the signing key list and the unused import, 8a5f12a the reference ordering, and f39892a the tar line and the apply_tool comment. Nothing earlier is outstanding.

@ptr727
ptr727 merged commit 56f4d7d into develop Aug 11, 2026
7 checks passed
@ptr727
ptr727 deleted the host-bootstrap-tooling branch August 11, 2026 03:54
ptr727 added a commit that referenced this pull request Aug 11, 2026
…ng (#670)

Five commits, `1927e9a..56f4d7d`. Nineteen files, +4241/-136.

## What lands

- **#664** `b0d0d13` — a shellcheck gate in `validate-task.yml`, with
the file list from `git ls-files '*.sh'` so a new script is gated
without editing the step. Also wired `scripts/test_host_gate.py` into
the self-test step, which was running in no workflow at all.
- **#666** `8c6fd27` — `prose_lint.py` chose its rule set from the
working directory rather than the scanned repository, so standing in an
operational repo and scanning a release repo silently discarded
`home-path`, the rule that exists because real paths reached a public
comment.
- **#665** `e2a99f1` — a host stamp at
`~/.claude/agent-safety-stamp.json` plus `--report`, so "is this machine
current" has an answer that is not a tick in an issue. Also fixed
`install.py` taking no arguments while both wrappers passed `"$@"`,
which made `--help` perform a full install.
- **#668** `6864a9b` — the two remaining `prose_lint.py` false cleans,
fixed as a class. An absolute path argument scoped a `--diff` run 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.
- **#667** `56f4d7d` — the host bootstrap tooling under
`host-setup/linux/`, its `bootstrap.sh` loader,
`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 reaches
`main`. 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

`develop` at `56f4d7d`, in sync with `origin/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.py` with 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 is `develop` itself.
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