Skip to content

dotfiles-autoupdate: run the ssh rewrite before the gantry CLI refresh - #133

Open
DJRHails wants to merge 2 commits into
mainfrom
autoupdate-gantry-after-ssh-rewrite
Open

dotfiles-autoupdate: run the ssh rewrite before the gantry CLI refresh#133
DJRHails wants to merge 2 commits into
mainfrom
autoupdate-gantry-after-ssh-rewrite

fix: resolve code review findings for PR #133

47de79e
Select commit
Loading
Failed to load commit list.
Sage Saint Review / Sage Saint Review succeeded Aug 23, 2026 in 40s

🟢 Sage review: approved

Direct single-pass review of PR #133 (7-line follow-up delta from PR #131's post-merge review; no agent fan-out). Verified the reorder is correct by reading modules/dotfiles-autoupdate/update.sh: ensure_github_ssh_rewrite only reads ~/.ssh/config and writes ~/.gitconfig.github-ssh (no reverse dependency on the gantry step), and it still runs after update_dotfiles so a freshly-pulled include directive is honored. One P2 finding: the ordering constraint was enforced only by a comment while tests/dotfiles-autoupdate.test.sh — the script's designated per-commit regression net — didn't pin it. Fixed in commit 47de79e: added a behavior test (uv stub records whether the rewrite file existed at 'uv tool install' time, stub_curl extended to serve the gantry /health version, keyed-host fixture). Break-checked: reverting the call order fails the new test with [missing]; note an initial sed-based break-check silently no-opped and was redone with explicit edits. Verification: full autoupdate suite passes (incl. 2 new checks), link-symlinks suite passes, shellcheck and bash -n clean on both touched scripts, all pre-commit hooks (gitleaks, trufflehog, glassine, 6 behavior suites) passed at commit time with no skips. Pushed 47de79e to the PR branch (the subsequent synchronize webhook was this push echoing back — no re-review). F1 thread replied to and resolved; summary comment posted with Verdict: approve.