fix(selfhost): distinguish missing-ref and detached-HEAD errors in update script - #4158
Conversation
…date script
selfhost-update.sh's ff-only merge failure previously printed the same
"diverged history" message for two unrelated causes: a genuine
non-fast-forward divergence, and a bad SELFHOST_UPDATE_BRANCH/
SELFHOST_UPDATE_REMOTE override pointing at a ref that doesn't exist at all
(git merge --ff-only fails the same way for both). Add an explicit
`git rev-parse --verify` check right after the fetch so a missing ref gets
its own distinct, more actionable error instead of being misreported as a
divergence. Also give a checkout in a detached HEAD state (where
`git rev-parse --abbrev-ref HEAD` literally returns "HEAD") its own message
instead of the generic branch-mismatch one, which read oddly ("currently on
'HEAD', expected 'main'") and suggested a nonsensical
SELFHOST_UPDATE_BRANCH=HEAD override.
Also fix an inaccurate comment in
test/unit/docs-selfhost-git-deploy-hygiene.test.ts's tracked-file-shadowing
test: .gitignore has no effect on files git already tracks (verified: `git
add -A` still stages a modification to an already-tracked-but-now-ignored
file). The real risk the test guards against is the opposite direction -- a
future PR adding a genuinely new tracked file whose name happens to match
the pattern would have it silently excluded from `git status`'s untracked
list and from `git add -A`/`git add .`, not "untracked on reclone" as the
comment previously and incorrectly claimed.
Add test coverage for both new error paths (detached HEAD, and
SELFHOST_UPDATE_BRANCH naming a branch the remote doesn't have) to
selfhost-update-script.test.ts.
Both nits were flagged by the Gittensory Orb review on #4151 (merged).
Closes #4156
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4158 +/- ##
=======================================
Coverage 93.73% 93.73%
=======================================
Files 386 386
Lines 36304 36304
Branches 13298 13298
=======================================
Hits 34029 34029
Misses 1618 1618
Partials 657 657 🚀 New features to boost your workflow:
|
loopover-orb
Bot
added
the
gittensor:bug
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-08 08:39:43 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 6 non-blocking
Linked issue satisfactionNot yet addressed Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
1 participant
Summary
Precision follow-up to #4151 (merged; #1660 itself is done). While that PR was open, the Gittensory Orb
review flagged 7 non-blocking nits (readiness 100/100, no blockers, held only because it touched
guarded paths). #4151 got merged by a separate verification pass before I finished triaging those
nits, so this PR carries the two genuinely worth fixing, now tracked under #4156 since issue #1660 already shows CLOSED
already closed:
scripts/selfhost-update.sh'sgit merge --ff-onlyfailure pathprinted the same "local history has diverged" message for two different causes: a real
non-fast-forward divergence, and a bad
SELFHOST_UPDATE_BRANCH/SELFHOST_UPDATE_REMOTEoverridepointing at a ref that doesn't exist at all (
git merge --ff-only <bad-ref>fails the same wayeither way). Added
git rev-parse --verify --quiet "$REMOTE/$BRANCH"right after the fetch so amissing ref gets its own distinct, actionable error instead of being misreported as a divergence.
current_branch="$(git rev-parse --abbrev-ref HEAD)"literally returns the string
HEADin a detached checkout, so the existing branch-mismatcherror read "currently on 'HEAD', expected 'main'" and suggested a nonsensical
SELFHOST_UPDATE_BRANCH=HEADoverride. Detached HEAD now gets its own message.Also fixed, alongside those two: an inaccurate comment on the
.gitignoretracked-file-shadowingtest (
test/unit/docs-selfhost-git-deploy-hygiene.test.ts) claiming a tracked file matching the new*.bak-*/*.backup-*patterns would get "silently untracked... the moment someone re-clones" —verified empirically (scratch repo: track a file, add a matching ignore pattern, modify it,
git add -A) that.gitignorehas no effect on already-tracked files at all; the modification stillstaged normally. The real risk runs the other way: a new file matching the pattern would be
silently excluded from
git status's untracked list and fromgit add -A/git add ., so it couldgo uncommitted without anyone noticing. Corrected the comment to describe that mechanism instead.
Closes #4156.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #4156).Validation
git diff --checknpm run actionlint(no workflow files touched; ran anyway as part of the full gate)npm run typecheck— cleannpm run test:coveragelocally — 587 test files passed / 2 skipped, 11982 tests passed / 7 skipped, unsharded. Nosrc/**/packages/**lines changed (onlyscripts/**,test/**), so no new Codecovcodecov/patchobligation — ran it anyway to confirm nothing broke.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderate— 0 vulnerabilitiestest/unit/selfhost-update-script.test.ts:SELFHOST_UPDATE_BRANCHnaming a branch the remote doesn't have gets the new "does not exist after fetching" message, distinct from the fast-forward-divergence message.20/20 passing (the 8 pre-existing cases plus the 2 new ones).
If any required check was skipped, explain why:
npm run test:cigate end-to-end (all steps green, includingtest:engine-parity,db:migrations:check,db:schema-drift:check,selfhost:env-reference:check,selfhost:validate-observability,cf-typegen:check,rees:test,ui:openapi:settings-parity,ui:version-audit,docs:drift-check,manifest:drift-check,command-reference:check,ui:test) beyond just the boxes above.gittensory-mcppre-submit predictors: same as chore(selfhost): script git-backed self-host updates, ignore stray backups #4151, they need an interactiveGitHub device-flow login this session can't complete, and as the repo owner this PR is held for
manual merge rather than auto-closed on an adverse gate signal.
Safety
ui:openapi:checkconfirms no drift.)scripts/**andtest/**.)UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository. (N/A — no UI/docs route touched by this PR.)UI Evidence
Not applicable: this PR only touches
scripts/selfhost-update.shand its test files — no UI, docsroute, or other visible surface.
Notes
the Orb's 7 nits (verifying which were real before pushing anything) when a separate verification
pass merged chore(selfhost): script git-backed self-host updates, ignore stray backups #4151 out from under the branch. Rather than push dangling commits to a closed PR's
branch, cut a fresh branch off the post-merge
origin/maintip, cherry-picked the one commit thatcarried genuine fixes, opened polish(selfhost): sharpen selfhost-update.sh error precision #4156 to track it (since issue build(selfhost): make host deploys Git-backed and image-ready #1660 already shows CLOSED and a linked open
issue is required), and rebased/re-verified the full gate against the new base before pushing.
.gitignore-doesn't-untrack claim above was checked in adisposable scratch git repo (track a file → add a matching ignore pattern → modify the file →
git add -A→ confirm it still stages) before rewriting the comment, rather than trusting eitherthe original (wrong) comment or the review's suggested rewording at face value — the suggested
rewording ("
git add -Awould silently skip the file going forward") turned out to also beimprecise for an already-tracked file, so the final comment states the mechanism for a new
file instead, which is what the test's
git ls-filescheck actually guards against.