Repository navigation
ci: add make ci-remote to run the pre-push gate's make ci elsewhere - #766
Closed
EricAndrechek wants to merge 5 commits into
Closed
EricAndrechek wants to merge 5 commits into
EricAndrechek wants to merge 5 commits into
Conversation
A code push needs tmp/ci-passed-tree-<tree>, and only a make ci in the
same checkout wrote it, so every push paid for a full local run and the
gate only ever exercised the contributor's own OS and architecture.
make ci-remote HOST=<ssh destination>|docker:<container> streams HEAD to
the host as a git bundle on the transport's stdin, checks it out in a
fresh directory, runs make tools && make ci there and streams the output
back. The same markers are written locally only if the host exits 0, the
tree it reports having checked out is HEAD^{tree}, make ci left that
checkout clean, and the local worktree is clean before and after the run
and still on that tree. Report lines carry a per-run nonce so make's own
output can't forge them. A provenance file beside the marker records
where the run happened, and the pre-push hook prints it.
scripts/ci/remote-ci.test.sh (a make verify leaf) runs the script through
stand-in ssh and docker commands in a scratch repository.
Closes #764. Part of #740.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stop the host run when the local side goes away: the host keeps its stdin open for the whole run and a watcher stops the run's process group when it closes. The bundle is length-prefixed so stdin can stay open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Interrupting make ci-remote now stops the run on the host, so say so in the development guide and the CHANGELOG entry. A host that runs make ci inside a container on its own network needs testcontainers pointed at the published ports. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A signal to the script alone left its keepalive loop, the transport and tee running, so the host's stdin never closed and make ci went on there with its run directory. The loop now also checks that the script is alive. The host's dirty check now passes --untracked-files=normal, as the local one does, so a host with status.showUntrackedFiles=no can't hide files a run left behind. Each run writes its own log, named with its nonce, so two runs of one tree don't overwrite each other's. The bundle is read through an open descriptor, so no temporary file outlives a killed run. Tests: signaling only the script, a report line forged in make's output, two runs of one tree, and every case under status.showUntrackedFiles=no. run() stops a hung run after 60s. The docs list curl, jq and Chromium's system libraries among the host's needs, and CONTRIBUTING.md counts a remote make ci. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used📚 Code guidelines (5)📓 Path-based instructions (6)Source excerpt: To add a command: drop a `.md` file in `.claude/commands/`.📄 CodeRabbit inference engine (docs/src/content/docs/claude-code.md) Files:
Source excerpt: Use [Conventional Commits](https://www.conventionalcommits.org/):📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
See [AGENTS.md](../AGENTS.md) for project conventions, architecture notes, and AI agent instructions.📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Source excerpt: **Never hard-wrap prose.📄 CodeRabbit inference engine (AGENTS.md) Files:
Source excerpt: **Docs prose**: never hard-wrap Markdown — one paragraph is one line.📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
Source excerpt: Run `make fix` to apply them.📄 CodeRabbit inference engine (docs/src/content/docs/development.md) Files:
🪛 ast-grep (0.45.3)scripts/ci/remote-ci.test.sh[warning] 129-129: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. (predictable-tmp-file-bash) 🪛 LanguageTooldocs/src/content/docs/development.md[locale-violation] ~353-~353: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects. (AFTERWARDS_US) [style] ~353-~353: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read. (TOO_LONG_SENTENCE) [style] ~357-~357: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional. (EN_REPEATEDWORDS_NEED) [style] ~357-~357: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read. (TOO_LONG_SENTENCE) CHANGELOG.md[style] ~51-~51: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read. (TOO_LONG_SENTENCE) 🔇 Additional comments (10)
📝 SummarySummary by CodeRabbit
WalkthroughAdds ChangesRemote CI
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Developer
participant Makefile
participant RemoteCIScript
participant SSHorDocker
participant RemoteMake
Developer->>Makefile: Run ci-remote with HOST
Makefile->>RemoteCIScript: Pass HOST, KEEP, and REMOTE_DIR
RemoteCIScript->>SSHorDocker: Stream bundle and remote runner
SSHorDocker->>RemoteMake: Run make tools and make ci
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This adds an opt-in make ci-remote command and lets the pre-push hook accept its markers. Markers are written only after a verified successful run on the exact tree, so the gate keeps its meaning. No merge-blocking issues were found. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 3 files. (7 skipped: 7 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
documentation
Member
Author
|
@coderabbitai review |
|
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
EricAndrechek
marked this pull request as ready for review
October 8, 2026 03:00
This was referenced Oct 8, 2026
Closed
Member
Author
EricAndrechek
added a commit
that referenced
this pull request
Oct 9, 2026
This PR has two parts: 1. **`make ci-remote`** runs the pre-push gate's `make ci` on another machine. 2. **The repo's hooks now fail closed.** These are git's `.githooks/` and the Claude Code hooks in `.claude/`. Each one failed the wrong way when a file it runs was missing, or when an action came in a form the hook didn't recognise (#678, #787, #790). They share the pre-push hook, the verify leaves and the hook docs, so they ship together and their overlaps are resolved once. ### Part 1: `make ci-remote` A code push needs `tmp/ci-passed-tree-<tree>`, and only a `make ci` in the same checkout wrote it. So every push paid for a full local run (race detector, coverage, a Docker daemon carrying every suite's containers), and the gate only ever exercised the contributor's own OS and architecture. `make ci-remote HOST=<host>` runs that `make ci` on another machine and keeps the gate's meaning. - **Transports.** `HOST` is any ssh destination (a `~/.ssh/config` alias works), or `docker:<container>` for a local container or devcontainer. Over ssh the commands run in a login shell; in a container, in its own environment. - **What travels.** The worktree must be clean. `HEAD` is streamed as a `git bundle` on the transport's stdin, right after the host-side script, so nothing is pushed. The host checks out that commit in a fresh directory under `REMOTE_DIR`, runs `make tools && make ci`, streams the output back with its `uname -sm` first, and removes the directory unless `KEEP=1`. Stopping the command, by Ctrl-C or a signal to the script alone, stops the run on the host: the host's stdin stays open only while the script runs, and a watcher stops the run's process group when it closes. - **When the marker is written.** Only if all of these hold: the host exits 0; the tree it reports having checked out is `HEAD^{tree}`; `make ci` left its checkout clean; and the local worktree is still clean and on that tree at the end. The host's report lines carry a per-run nonce, so `make ci`'s own output can't forge them. The verify marker is written too, as `make ci` does. - **Provenance.** `tmp/ci-passed-tree-<tree>.provenance` records the host, OS/arch, start and end times, and the run's own local copy of the log (`tmp/ci-remote-<tree>-<run>.log`, so concurrent runs of one tree don't overwrite each other). The pre-push hook prints `make ci passed on <host> (<os>/<arch>)` from it. Marker semantics and local `make ci` are unchanged. - **Docs.** A `make ci-remote` section in the development guide (usage, host requirements, how the gate treats it), the AGENTS.md pre-push rules, the hook table, and CONTRIBUTING.md's pre-push sentence. ### Part 2: hooks that fail closed - **#678:** git's `pre-commit` failed loudly in a worktree with no code, so nothing could be committed there. - **#787:** the Claude Code hooks failed silently when their scripts were gone, so the push gate stopped gating and nothing said so. - **#790:** the agent gate checked only the `gh pr` spelling of each action it guards, so the alias and API forms went through. Its `gh` checks also blocked commands that only *mentioned* a guarded action in a heredoc or a comment. Each now fails closed and says why. #### #678: git hooks in a worktree with no Makefile With an absolute `core.hooksPath`, git runs one checkout's `.githooks/` in every worktree, including one on an orphan branch that keeps a tool's state and no code. There, `pre-commit` couldn't find `scripts/ci-marker.sh`, hid that behind `2>/dev/null`, and ran `make verify` with no Makefile (`No rule to make target 'verify'`). - **`pre-commit`:** no `Makefile` in the worktree means nothing to verify, so it exits 0 with a note. A missing `scripts/ci-marker.sh`, or one that fails with anything but "no marker" (exit 1), is now reported, and then `make verify` runs, which is the fail-closed path. - **`pre-push`** had the same failure, reported as a misleading "can't resolve tree". It now decides per pushed commit, not per worktree. A pushed commit with no `Makefile` holds no code to validate, so it goes through. One with a `Makefile`, pushed from a worktree that lacks `scripts/ci-marker.sh`, is blocked with a message naming the script. Deciding by worktree would have failed open: `git push origin <code-branch>` from a worktree with no code would have skipped the check. - **The `make ci-remote` test needed one fix for this.** Its scratch repository had no `Makefile`. So once the pre-push hook passed a commit without one, the test's two pre-push cases ("blocks a code push with no marker" and "accepts the marker and names the host") stopped exercising the marker check and failed. A code repository has a Makefile, so the scratch one now does too, in its own commit. **On `core.hooksPath`.** This PR doesn't change it. `make tools` sets the relative `.githooks`, and git resolves a relative `core.hooksPath` in each worktree (checked with git 2.56). So each worktree runs its own branch's hooks, and a worktree whose branch has no `.githooks/` runs no hooks at all. That fails open, the class of bug #787 is about. #678 was found with an absolute path, which runs one checkout's hooks everywhere. Switching to a relative path would have "fixed" #678 by turning a loud failure into a silent one. So the fix is in the hooks, which now behave correctly under either setting. #### #787: Claude Code hooks whose scripts are missing Each hook in `.claude/settings.json` was a bare `$CLAUDE_PROJECT_DIR/.claude/hooks/<script>.sh`. `CLAUDE_PROJECT_DIR` is fixed when the session starts. Once that checkout is removed, or a script loses its executable bit, the shell exits 127 or 126. Claude Code [treats that as a non-blocking error](https://code.claude.com/docs/en/hooks#other-exit-codes): "When the script path doesn't exist or isn't executable, the shell exits with a code like 127 … For most hook events, the action proceeds. … a mistyped path in `settings.json` leaves the gate silently disabled." The scripts may be the thing that's missing, so the fix has to live in the commands. Each is now a one-line POSIX `sh` guard; `sh -c` is how Claude Code [runs a shell-form hook](https://code.claude.com/docs/en/hooks#exec-form-and-shell-form) on macOS and Linux. When the script is there and executable, the guard `exec`s it, with the same process, stdin, argv and environment as before. The only visible change is that the path is now quoted, so a project directory with a space in it works. When the script isn't there, the guard acts by event: | Event | Script missing or not executable → | Why this code | | ----- | ---------------------------------- | ------------- | | PreToolUse (Bash) | **exit 2** for any command containing the words `git` and `push`; `gh` and `pr` with one of `create`, `new`, `edit`, `ready`, `review` or `merge`; or `gh` and `api` with `pulls` or `graphql`. That last rule blocks every call to the pull-request or GraphQL API, reads included. Also **exit 2 for every command** when `jq` is missing or the payload can't be parsed. Every other command: exit 0. | "Exit 2 means a blocking error … `PreToolUse` blocks the tool call", and exit 2 is "the only exit code that blocks through the code alone" ([Exit code 2](https://code.claude.com/docs/en/hooks#exit-code-2)). | | SubagentStop | **exit 1**, with a one-line stderr message | For SubagentStop, exit 2 "Prevents the subagent from stopping" ([per-event table](https://code.claude.com/docs/en/hooks#exit-code-2-behavior-per-event)), which risks a loop. A non-blocking error shows "a `<hook name> hook error` notice followed by the first line of stderr" ([Other exit codes](https://code.claude.com/docs/en/hooks#other-exit-codes)), so the message is one line. No marker gets written, so a push is still blocked, by the push gate or by the PreToolUse fallback above. | | PostToolUse (both formatters) | **exit 2**, with a stderr message | For PostToolUse, exit 2 "Shows stderr to Claude; the tool already ran" ([per-event table](https://code.claude.com/docs/en/hooks#exit-code-2-behavior-per-event)). The docs recommend exit 2 for exactly this case: "To surface a warning to Claude from a `PostToolUse` … hook, exit 2 instead" ([Exit code 0](https://code.claude.com/docs/en/hooks#exit-code-0)). Stderr from exit 0 "goes to the debug log only … Claude never sees it", and exit 1 only reaches the transcript notice. | Each message names the missing script and says how to recover: restart Claude Code from a live checkout. Design notes: - **Why not block every Bash call?** A session whose launch checkout has gone can still be working in other worktrees, and it needs Bash to notice the problem and recover. Blocking only what the gate guards keeps the gate's guarantee. The exception is a missing `jq`: then the command can't be read, so every command is blocked, which is what `agent-bash-gate.sh` already does without `jq`. - **The fallback's detection is crude on purpose.** It matches whole words anywhere in the command text, ignoring quoting, so it over-blocks: `git stash push`, `git push` mentioned in a commit message, and a read-only `gh api …/pulls/N/comments` are all blocked. It never runs while the real gate can run, and like the gate it guards against accidents, not deliberate workarounds. Only `.tool_input.command` decides; the Bash tool's `description` field is ignored. - **`jq` isn't guaranteed.** It's a documented prerequisite (`make verify` and the hooks need it), not a given, so its absence fails closed. - **Every fallback reads its payload to the end** before exiting, so Claude Code isn't left half-way through writing a large `Write` payload to a hook that never reads it. - **A script that exists but can't start** (a broken shebang, say) still fails open. The guard checks that the script exists and is executable; catching a failed `exec` would mean giving up the `exec`. #### #790: the agent gate missed the alias and API forms of what it guards `agent-bash-gate.sh` matched only the `gh pr` spellings, so each guarded action went through in another form: - `gh pr new`, an alias of `gh pr create`, skipped the `--draft` check and the title lint. - `gh api …/pulls/N/reviews -f event=APPROVE` approved a PR. - `gh api …/pulls -f head=… -f title=…` opened a non-draft PR. - `gh api …/requested_reviewers -f 'reviewers[]=…'` requested a reviewer. The check wanted `-X POST` or a literal `reviewers=`, though a field already makes `gh api` POST. - `gh api -X PUT …/pulls/N/merge` merged. - GraphQL `markPullRequestReadyForReview` marked a draft ready. Each of these now gets the same answer as its `gh pr` form: - **`gh pr new`** is checked like `gh pr create`. - **REST.** A `gh api` call counts as a write when it names a method other than GET or, naming none, sends fields. - A write to `…/requested_reviewers` (`DELETE` included) is blocked. - A write to `…/pulls/N/merge` is blocked. - A write that opens a PR (`…/pulls`) needs `draft=true`. Its `title=` goes through the PR-title lint, as does a retitle (`PATCH …/pulls/N`). - A review with `event=APPROVE` or `event=REQUEST_CHANGES` is blocked. - **GraphQL.** These are blocked: - `markPullRequestReadyForReview`; - `mergePullRequest` and `enablePullRequestAutoMerge`; - `requestReviews`; - an `addPullRequestReview` or `submitPullRequestReview` that approves or requests changes; - a `createPullRequest` without `draft: true`. - **`gh pr merge` stays with the `deny` rule** in `.claude/settings.json` and out of the gate. A person merges with it from Claude Code's shell mode (`! gh pr merge …`), and the docs don't say whether PreToolUse hooks see shell-mode commands. The [shell-mode section](https://code.claude.com/docs/en/interactive-mode#shell-mode-with-prefix) says only that they run "without going through Claude" and don't "require Claude to interpret or approve the command"; the [hooks reference](https://code.claude.com/docs/en/hooks) doesn't mention them. Blocking it in the gate could break that workflow. The API and GraphQL merge forms don't overlap with it, so the gate blocks them. **The checks now read what the line runs, not what it mentions.** The `gh` checks used to read the line with quoted strings removed and everything else kept. That made them wrong both ways: - They blocked a command that only mentioned a guarded action in a heredoc body or a comment, such as a `python3 - <<'EOF'` script with `# gh pr ready` in it. - They missed a guarded action inside a `$(…)` or handed to `bash -c`. They now read the commands the line runs, through the gate's own tokenizer, the same view the push check already uses: - A mention in a quoted string, a heredoc body or a comment doesn't count. - Code a shell runs is queued, tokenized and judged the same way. That covers a `$(…)`, `eval`'s words, a heredoc a shell reads directly or through a pipe, and the code a shell is handed with `-c`. Options before `-c` are stepped over, including the value of `-o`/`+o`: `bash -euo pipefail -c '…'` and `bash --noprofile --norc -c '…'` both count. A short-option cluster containing `c` counts as `-c`; a long option such as `--norc` doesn't. - The code inside a `$(…)` is read as written, quotes and heredoc bodies included. So `url=$(gh pr create --draft --title "feat(x): …")` is judged on its real title, and a heredoc a shell reads inside a `$(…)` counts too. (The push check still doesn't follow that last form.) - A GraphQL `query=` computed by a command (`-f query="$(cat <<'EOF' … EOF)"`) has its guarded mutation names matched against the code that computes it, heredoc bodies included. - The gate follows 64 pieces of nested code. Past that, a piece left that mentions a `gh pr` or `gh api` command is refused, unjudged. - A line the tokenizer can't parse is refused when it mentions a `gh pr` or `gh api` command, because what it runs can't be told. **Each `gh` command is judged on its own words.** - A `gh api` call is judged by its endpoint (its first argument that isn't a flag) and by its fields. `draft=`, `title=` and `event=` are read only from `-f`, `-F`, `--field` or `--raw-field` values, and GraphQL mutation names only from the `query=` field. So a review-thread reply or comment whose body names an endpoint, a field or a mutation doesn't count, and neither does another call on the same line. - gh's own `-R`/`--repo` before the command is stepped over (`gh -R o/r pr ready 12`, `--repo=o/r`, `-Ro/r`), for `pr` and `api` both. - A boolean flag set with `=` is read as gh reads it: `--draft=true` counts as `--draft`, and `--approve=false` doesn't approve. - Titles are read from the tokens, so an unquoted `--title` is linted too. The gate still reads the command line only, so fields sent through `--input` or `-F field=@file` go unseen. **It runs under macOS's bash 3.2.** On a Mac with no newer bash on `PATH`, `#!/usr/bin/env bash` finds `/bin/bash` 3.2. An earlier revision of this branch stopped there with "unbound variable" whenever a gh line had no substitution, because it expanded an empty array under `set -u`. The gate then exited 1, which Claude Code doesn't treat as a block. The scan now walks its arrays by index. The three hook tests run a second time with `/bin/bash` first on `PATH` when it is a different bash, so a construct 3.2 rejects fails a test. **Timing.** The tokenizer used to step through a double-quoted string one character at a time; it now takes runs of plain text in one step. Measured on an earlier revision of this change (a 100 KB inline `--body "…"`: 11 s with the old character loop, under 1 s after); not re-measured after the per-call rewrite. ## Test plan **On this branch at its head, 9b8c30c (measured):** - [x] `make verify` passes with all 23 leaves. - [x] `make build-docs` passes. - [x] shellcheck v0.11.0 is clean on all 48 tracked `*.sh` and `.githooks/*` files. - [x] The pre-commit `make verify` passed on each commit that went through it. - [ ] `make ci`: not run. **Part 1, `make ci-remote`.** Carried over from the stacked version of this PR (#766) unless marked as re-run here. - [x] `scripts/ci/remote-ci.test.sh`, a new `make verify` leaf (`test-remote-ci`): **31/31 on this branch (re-run here)**. It runs in a scratch repository, which now has a Makefile, with stand-in `ssh`/`docker` commands that run the host side for real, in its own process group as sshd would, against a stub `make`, `uname` and `git`. It covers: - a passing run: markers, provenance fields, the log, host cleanup; - `KEEP` and `REMOTE_DIR`; - a failing `make ci`; - two tree mismatches: HEAD moved during the run, and the host checked out another commit; - a worktree dirtied during the run, and `make ci` dirtying the host's checkout; - stopping the local process group, or only the script, mid-run; either way the host's `make ci` stops and its directory goes; - a report line forged in `make ci`'s output; - dirty-worktree and bad-`HOST` refusals with `status.showUntrackedFiles=no` set; - the `docker exec` command line; - two runs of one tree keeping separate logs; - the pre-push hook with and without provenance. - [ ] Each check in the script and the hook was deleted in turn, and every deletion fails at least one case. The exception was the empty-report check, which was then folded into the tree comparison. (Carried over.) - [ ] Against a real Linux x86_64 host over ssh (carried over): - What worked: the bundle checked out at the right tree, the report lines came first, and phase 1 of `make ci` passed there, `remote-ci test` included. - What didn't: phase 2 needed host-side Docker networking setup. `make ci` ran inside a container on the host's network, so testcontainers dialed the bridge gateway and timed out; `TESTCONTAINERS_HOST_OVERRIDE=localhost` is the documented fix. - No marker was written for that run, and none when the local side was killed. - A full green `make ci` on a remote host has not been run yet. - [ ] `ssh -T` and `docker exec -i` both return within a second of the remote command exiting while local stdin is still open, which the interrupt watcher relies on. (Carried over.) **Part 2, hooks that fail closed (measured):** - [x] `make test-githooks` (`scripts/githooks.test.sh`, new verify leaf): 11/11 pass, under bash 5.3 and again with `/bin/bash` 3.2 first on `PATH`. - It builds a scratch repository whose `core.hooksPath` names this checkout's `.githooks/` by absolute path, then commits and pushes from a code worktree and from an orphan worktree with no Makefile. - The test clears git's repository variables, uses `git -C` throughout, and refuses to write unless each scratch repository and worktree is where it asked. - Against the old hooks, **6 of 11 fail**, including `make: *** No rule to make target 'verify'` for the commit with no Makefile. - [x] `make test-hook-commands` (`.claude/hooks/hook-commands.test.sh`, new verify leaf): 171/171 pass under `sh`, `dash` and `bash`, and 171/171 again with `/bin/bash` 3.2 first on `PATH`. - It reads every hook command from the real `settings.json` and runs it under each shell. - It covers an empty `CLAUDE_PROJECT_DIR`, a non-executable copy of every script, the variable unset, and no `jq`. - It runs each command against stub scripts that record their stdin byte for byte, their argv and their exit code, including with a path containing a space. - It also runs this checkout's real scripts. - A catch-all case fails any hook command, present or future, that exits 0, 126 or 127 when its script is missing or not executable. Dropping `[ -x "$h" ] &&` from the SubagentStop guard fails 6 cases. - Against the old `settings.json`, **141 of 171 fail**. The 30 that pass are exactly the stub-script and real-checkout cases, which shows the commands behave as before when the scripts exist. - [x] `make test-review-gate`: 230/230 pass under bash 5.3, and 230/230 with `/bin/bash` 3.2.57 first on `PATH`. - The 93 new cases cover every alias, REST and GraphQL form above. - **Code inside `$(…)`, read as written:** - Blocked: a GraphQL ready mutation captured by `$(…)`; a bad title is still linted there; a `gh pr ready` in a heredoc read by `bash` inside `$(…)`. - Allowed: a draft PR's URL captured with a two-word title and with a scoped title; a comment body with an apostrophe; a `grep -c 'gh pr ready'`. - **Look-alikes that must go through:** GET on a PR, its reviews, its requested reviewers and its merge status; `-X GET` with fields; a COMMENT review; a GraphQL query; a GraphQL draft create; an issue comment that mentions approving and merging. - **Mentions, allowed:** a Python heredoc's comment, a `cat` heredoc, a shell comment, a quoted string, a PR-body heredoc. - **A guarded command a shell runs, blocked:** in a `$(…)`, `bash -c`, `bash -lc`, `sh -c` or `eval`, in a heredoc read by `bash` or piped into `sh`, and behind `timeout`. - **`=`-flags in both directions:** `--draft=true` allowed, `--draft=false` blocked, `--approve=true` and `--request-changes=true` blocked, `--approve=false --comment` allowed. - **Per-call judgment:** a review-thread reply quoting a merge endpoint, a comment naming `requested_reviewers` or `event=APPROVE`, and a GraphQL comment naming a mutation are allowed. A reviewer read followed by a comment is allowed; a comment followed by a reviewer write is blocked. A non-draft create whose body says `draft=true` is blocked. - **`-R`/`--repo` forms:** blocked for ready, review `--approve`, create without `--draft`, edit `--add-reviewer` and an API merge. `gh -R o/r pr view` is allowed. - **Nesting cap:** a `gh pr ready` in the 63rd `$(…)` is judged and blocked, one past the 64 pieces the gate follows is refused, and a gh read beside 70 substitutions with no gh in them is allowed. - **Heredoc GraphQL query:** ready-for-review is blocked; a read is allowed. - **Shell options before `-c`:** `-e -c`, `-euo pipefail -c`, `-o pipefail -c`, `+o posix -c` and `--noprofile --norc -c` are all followed, by the gh check and the push check alike. `bash --norc` running a script and a read under `bash -o pipefail -c` are allowed. - **Against main's gate:** 49 of the 93 fail, under both bashes. - **Against earlier revisions of this branch:** - The revision before the last round failed 6 of the 7 newest cases, the `$(…)` ones above, under both bashes. Only "a bad title is still linted" passed there. - The round before that failed 12 of its 21 newest cases (cap, `-R`, heredoc query, shell options), under both bashes. - The one before that, under `/bin/bash` 3.2, gave 135 ok / 67 FAIL, every failure "TK_SUBS[@]: unbound variable". - [x] `make test-hook-commands` also caught the bash 3.2 bug: "the real gate blocks gh pr ready" exited 1 instead of 2, in all three shells. - [x] The inline guards pass shellcheck as `sh` and `dash`. `make lint-sh` now also checks the extensionless `.githooks/` files. - [x] Timing: measured on an earlier revision of this change (a 100 KB double-quoted `--body`: 11 s with the old character loop, under 1 s after); not re-measured after the per-call rewrite. ## Related Issues Closes #764 Closes #678 Closes #787 Closes #790 Supersedes #766 (rebuilt on `main` after #785). Part of #740 Based on `main`. The two PRs it was stacked on, #793 (`-trimpath`) and #794 (the per-PR build cache), have merged. Gate forms review found but left out of this PR, as less common than the ones it covers: #797. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Wjv2nu841pxTMdUWSAPmDH --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Summary
A code push needs
tmp/ci-passed-tree-<tree>, and only amake ciin the same checkout wrote it. So every push paid for a full local run (race detector, coverage, a Docker daemon carrying every suite's containers), and the gate only ever exercised the contributor's own OS and architecture.make ci-remote HOST=<host>runs thatmake cion another machine and keeps the gate's meaning.HOSTis any ssh destination (a~/.ssh/configalias works), ordocker:<container>for a local container or devcontainer. Over ssh the commands run in a login shell; in a container, in its own environment.HEADis streamed as agit bundleon the transport's stdin, right after the host-side script, so nothing is pushed. The host checks out that commit in a fresh directory underREMOTE_DIR, runsmake tools && make ci, streams the output back with itsuname -smfirst, and removes the directory unlessKEEP=1. Stopping the command, by Ctrl-C or a signal to the script alone, stops the run on the host: the host's stdin stays open only while the script runs, and a watcher stops the run's process group when it closes.HEAD^{tree},make cileft its checkout clean, and the local worktree is still clean and on that tree at the end. The host's report lines carry a per-run nonce, somake ci's own output can't forge them. The verify marker is written too, asmake cidoes.tmp/ci-passed-tree-<tree>.provenancerecords the host, OS/arch, start and end times, and the run's own local copy of the log (tmp/ci-remote-<tree>-<run>.log, so concurrent runs of one tree don't overwrite each other). The pre-push hook printsmake ci passed on <host> (<os>/<arch>)from it. Marker semantics and localmake ciare unchanged.make ci-remotesection in the development guide (usage, host requirements, how the gate treats it), the AGENTS.md pre-push rules, the hook table, and CONTRIBUTING.md's pre-push sentence.Test plan
scripts/ci/remote-ci.test.sh, a newmake verifyleaf (test-remote-ci): 31 cases in a scratch repository, with stand-inssh/dockercommands that run the host side for real, in its own process group as sshd would, against a stubmake,unameandgit. It covers a passing run (markers, provenance fields, the log, host cleanup),KEEP/REMOTE_DIR, a failingmake ci, two tree mismatches (HEAD moved during the run; the host checked out another commit), a worktree dirtied during the run,make cidirtying the host's checkout, stopping the local process group or only the script mid-run (either way the host'smake cistops and its directory goes), a report line forged inmake ci's output, dirty-worktree and bad-HOSTrefusals withstatus.showUntrackedFiles=noset, thedocker execcommand line, two runs of one tree keeping separate logs, and the pre-push hook with and without provenance.make lint-sh lint-md lint-prose test-classify-pathspass; pre-commitmake verifypassed on the commit.make cipassed there,remote-ci testincluded. Phase 2 needed host-side Docker networking setup:make ciran inside a container on the host's network, so testcontainers dialed the bridge gateway and timed out (TESTCONTAINERS_HOST_OVERRIDE=localhostis the documented fix). No marker was written for that run, and none when the local side was killed. A full greenmake cion a remote host has not been run yet.ssh -Tanddocker exec -iboth return within a second of the remote command exiting while local stdin is still open, which the interrupt watcher relies on.Related Issues
Closes #764. Part of #740. Stacked on #765 (base
build/trimpath); contains the whole stack below it and today'smain.🤖 Generated with Claude Code