chore: single-source the CI checks and share one guarded git mock - #186
Merged
st0nefish-ci[bot] merged 1 commit intoAug 29, 2026
Merged
Conversation
Two near-misses from the last session had the same shape: something was maintained in two places, the copies drifted, and the drift failed silently. **The check list.** CLAUDE.md said "CI runs four independent checks" and listed four; CI runs five. The missing one, shell linting, lived inline in the workflow YAML, so there was no way to reproduce it locally at all — you could pass every documented check and still fail CI. The markdown entry was also `rumdl .`, which is not a valid invocation and linted nothing. - Extract the inline shellcheck job into .github/scripts/lint-shell.sh and have the workflow call it, so CI is reproducible locally - Add .github/scripts/validate-all.sh running all five with a pass/fail summary, and point CLAUDE.md at that single command instead of a list that can rot independently - lint-shell.sh treats a missing shellcheck as a warning locally but a hard failure when CI is set, so a broken runner image cannot go silently green **The git mock.** It was hand-copied into nine suites, and every copy delegated with `command git "$@"`. `command` bypasses functions and aliases but not PATH lookup, and the mock's directory is prepended to PATH, so each copy re-executed itself without bound. - Add tests/lib/mock-git.sh with a single write_mock_git, guarded twice: delegation strips the mock's own directory from PATH, and a depth counter aborts on the third re-entry so a future delegation mistake fails loudly instead of forking forever - Convert all nine suites to it, removing 119 lines of duplication - lint-shell.sh fails the build if `command git|gh|tea` reappears in tests/ Verified rather than assumed: the depth guard fires (exit 1, "recursion detected ... depth 3") against a deliberately broken delegation; the lint backstop both passes clean and rejects a reintroduced mock; validate-all.sh propagates failure (exit 1) rather than always succeeding; suite discovery still finds 32 suites and ignores the new lib. validate-all.sh: all five checks pass, 1857 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EddJxaTMXyj7o4ndrqfCxa
There was a problem hiding this comment.
🟡 Changes recommended
The new shared git mock’s recursion guard is off-by-one versus the documented/tested behavior, which should be corrected to ensure the safety mechanism triggers at the intended depth.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Consolidates CI validation into repo scripts and centralizes the test git mock to prevent silent drift and recursive PATH delegation failures, making CI checks reproducible locally and reducing duplicated test harness code.
Changes:
- Extract shell linting logic into
.github/scripts/lint-shell.shand add.github/scripts/validate-all.shas the single local entrypoint for all CI checks. - Introduce
tests/lib/mock-git.sh(write_mock_git) and update affected test suites to use it instead of hand-copied PATH-injected mocks. - Update
CLAUDE.mdand CI workflow to reference the new scripts and standardized test mock approach.
File summaries
| File | Description |
|---|---|
| tests/session/test-pr-wait.sh | Switches to shared write_mock_git instead of inlined git mock. |
| tests/session/test-pr-auto-merge-status.sh | Switches to shared write_mock_git instead of inlined git mock. |
| tests/session/test-ci-poll.sh | Replaces multiple inlined git mocks with write_mock_git calls. |
| tests/lib/mock-git.sh | Adds shared git mock generator with recursion guard and safe delegation. |
| tests/git-cli/test-run-watch-gitea.sh | Uses shared git mock generator with an additional rev-parse arm. |
| tests/git-cli/test-run-show.sh | Switches to shared write_mock_git for platform detection. |
| tests/git-cli/test-pr-show-gitea.sh | Switches to shared write_mock_git for platform detection. |
| tests/git-cli/test-pr-create.sh | Switches to shared write_mock_git and adds extra case arm. |
| tests/git-cli/test-issue-write-json.sh | Uses shared mock for platform switching between GitHub/Gitea. |
| tests/git-cli/test-body-args.sh | Switches to shared write_mock_git and adds extra case arm. |
| CLAUDE.md | Documents validate-all.sh and standardized git mock guidance for tests. |
| .github/workflows/ci.yml | Replaces inline shell lint step with .github/scripts/lint-shell.sh. |
| .github/scripts/validate-all.sh | New script to run all CI checks locally with summary and proper exit code. |
| .github/scripts/lint-shell.sh | New shellcheck runner + backstop for detecting unsafe command <tool> delegation in tests. |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+35
to
+40
| if [ "${MOCK_GIT_DEPTH:-0}" -gt 2 ]; then | ||
| echo "mock git: recursion detected on '$*' (depth ${MOCK_GIT_DEPTH})" >&2 | ||
| exit 1 | ||
| fi | ||
| export MOCK_GIT_DEPTH=$((${MOCK_GIT_DEPTH:-0} + 1)) | ||
| case "$*" in |
Comment on lines
+23
to
+27
| # # with extra cases (one `case` arm per line, matched before delegation): | ||
| # write_mock_git "$MOCK_DIR" "https://github.com/owner/repo.git" \ | ||
| # ' "config user.name") echo "testuser" ;;' | ||
|
|
||
| # write_mock_git <mock_dir> <remote_url> [extra_case_arms] |
st0nefish-ci
Bot
deleted the
chore/single-source-validation-and-mock-helper
branch
August 29, 2026 22:10
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
Follow-up to #184 and #185. Both of those were near-misses with the same shape: something maintained in two places, the copies drifted, and the drift failed silently. This fixes the mechanism rather than another instance.
1. The check list was maintained twice and drifted twice
CLAUDE.mdsaid "CI runs four independent checks" and listed four. CI runs five. The missing one — shell linting — lived inline in the workflow YAML, so there was no way to reproduce it locally at all: you could pass every documented check and still fail CI. The markdown entry was alsorumdl ., which is not a valid invocation and linted nothing (fixed in #185)..github/scripts/lint-shell.sh; the workflow now calls it.github/scripts/validate-all.shrunning all five with a pass/fail summaryCLAUDE.mddocuments that one command; the per-check list stays as a reference for running one in isolationlint-shell.shtreats missing shellcheck as a warning locally but a hard failure whenCIis set, so a broken runner image can't go silently greenCI keeps its five parallel jobs — per-job status is worth having.
2. The git mock was hand-copied into nine suites
Every copy delegated with
command git "$@".commandbypasses functions and aliases but not PATH lookup, and the mock's directory is prepended to PATH — so each copy re-executed itself without bound. Latent until something called a second git subcommand (#184'srev-parse), then 56k processes and a dead terminal.tests/lib/mock-git.shprovides onewrite_mock_git, guarded twice: delegation strips the mock's own directory from PATH, and a depth counter aborts on the third re-entry so a future delegation mistake fails loudly instead of forking foreverlint-shell.shfails the build ifcommand git|gh|teareappears undertests/Test plan
Verified rather than assumed:
command gitdelegation it exits 1 withmock git: recursion detected on 'rev-parse HEAD' (depth 3), no process growthvalidate-all.shpropagates failure: forced a check to fail, confirmed exit 1 — a validator that always exits 0 is worse than nonetests/lib/mock-git.shcorrectly ignored (discovery matchestest-*.shundertests/*/)validate-all.sh: all five checks pass, 1857 tests