mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-27 11:22:03 +00:00
fix(desktop): don't report a bogus update count for a shallow checkout
The desktop installer clones with `--depth 1`, so a public install's local history often shares no merge-base with the freshly fetched origin tip. In that state `git rev-list HEAD..origin/<branch> --count` enumerates the entire remote ancestry and returns a meaningless huge number, surfacing as e.g. "v0.17.0 (+12104)" in the update indicator (#51922). The official-SSH branch of checkUpdates() already sidesteps this by reporting a binary up-to-date check (`behind: currentSha === targetSha ? 0 : 1`), and hermes_cli/banner.py guards the identical class for the CLI banner. The passive desktop count path was the one place the shallow guard was missing. Detect shallow + no-merge-base up front and fall back to the same SHA-based binary check; full clones (developers / Docker dev images) keep the exact count path unchanged. The resolution logic lives in a pure update-count.cjs helper so it is unit-testable without booting Electron.
This commit is contained in:
parent
d335164833
commit
a6485bddb8
4 changed files with 84 additions and 4 deletions
|
|
@ -57,6 +57,7 @@ const {
|
|||
const { gitRootForIpc } = require('./git-root.cjs')
|
||||
const { worktreesForIpc } = require('./git-worktrees.cjs')
|
||||
const { OFFICIAL_REPO_HTTPS_URL, isOfficialSshRemote } = require('./update-remote.cjs')
|
||||
const { resolveBehindCount } = require('./update-count.cjs')
|
||||
const { runRebuildWithRetry } = require('./update-rebuild.cjs')
|
||||
const {
|
||||
buildPosixCleanupScript,
|
||||
|
|
@ -1706,15 +1707,25 @@ async function checkUpdates() {
|
|||
}
|
||||
|
||||
const git = args => runGit(args, { cwd: updateRoot }).then(r => r.stdout.trim())
|
||||
const [currentSha, targetSha, countStr, dirtyStr, currentBranch] = await Promise.all([
|
||||
const [currentSha, targetSha, countStr, dirtyStr, currentBranch, shallowStr, mergeBaseStr] = await Promise.all([
|
||||
git(['rev-parse', 'HEAD']),
|
||||
git(['rev-parse', `origin/${branch}`]),
|
||||
git(['rev-list', `HEAD..origin/${branch}`, '--count']),
|
||||
git(['status', '--porcelain']),
|
||||
git(['rev-parse', '--abbrev-ref', 'HEAD'])
|
||||
git(['rev-parse', '--abbrev-ref', 'HEAD']),
|
||||
git(['rev-parse', '--is-shallow-repository']),
|
||||
// merge-base exits non-zero with empty stdout when HEAD shares no common
|
||||
// ancestor with the freshly fetched tip — exactly the shallow-clone case.
|
||||
git(['merge-base', 'HEAD', `origin/${branch}`])
|
||||
])
|
||||
|
||||
const behind = Number.parseInt(countStr, 10) || 0
|
||||
const behind = resolveBehindCount({
|
||||
countStr,
|
||||
currentSha,
|
||||
targetSha,
|
||||
isShallow: shallowStr === 'true',
|
||||
hasMergeBase: Boolean(mergeBaseStr)
|
||||
})
|
||||
const commits = behind > 0 ? await readCommitLog(updateRoot, branch) : []
|
||||
|
||||
return {
|
||||
|
|
|
|||
20
apps/desktop/electron/update-count.cjs
Normal file
20
apps/desktop/electron/update-count.cjs
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
'use strict'
|
||||
|
||||
// Resolve how many commits the local checkout is behind origin for the desktop
|
||||
// update indicator. On a SHALLOW checkout (installer clones with --depth 1) the
|
||||
// local history often shares no merge-base with the freshly fetched origin tip,
|
||||
// so `git rev-list HEAD..origin/<branch> --count` enumerates the entire remote
|
||||
// ancestry and returns a bogus huge number (e.g. 12104) — see #51922. In that
|
||||
// case the count is meaningless; fall back to a binary up-to-date check by SHA,
|
||||
// exactly like the official-SSH path in checkUpdates() and the CLI guard in
|
||||
// hermes_cli/banner.py. Full clones (developers / Docker dev images) keep the
|
||||
// exact count path unchanged.
|
||||
function resolveBehindCount({ countStr, currentSha, targetSha, isShallow, hasMergeBase }) {
|
||||
if (isShallow && !hasMergeBase) {
|
||||
if (currentSha && targetSha && currentSha === targetSha) return 0
|
||||
return 1 // behind by an unknown amount — show a generic "update available"
|
||||
}
|
||||
return Number.parseInt(countStr, 10) || 0
|
||||
}
|
||||
|
||||
module.exports = { resolveBehindCount }
|
||||
49
apps/desktop/electron/update-count.test.cjs
Normal file
49
apps/desktop/electron/update-count.test.cjs
Normal file
|
|
@ -0,0 +1,49 @@
|
|||
'use strict'
|
||||
const test = require('node:test')
|
||||
const assert = require('node:assert/strict')
|
||||
const { resolveBehindCount } = require('./update-count.cjs')
|
||||
|
||||
// FAIL-BEFORE: pre-fix the function did `Number.parseInt(countStr) || 0`
|
||||
// unconditionally, so a shallow checkout with no merge-base surfaced the bogus
|
||||
// rev-list count (e.g. 12104). This asserts the new shallow/no-merge-base branch.
|
||||
test('shallow checkout with no merge-base does NOT trust the bogus rev-list count', () => {
|
||||
assert.equal(resolveBehindCount({
|
||||
countStr: '12104', currentSha: 'aaa', targetSha: 'bbb',
|
||||
isShallow: true, hasMergeBase: false,
|
||||
}), 1)
|
||||
})
|
||||
|
||||
test('shallow checkout with no merge-base but identical SHA reports up-to-date', () => {
|
||||
assert.equal(resolveBehindCount({
|
||||
countStr: '12104', currentSha: 'abc', targetSha: 'abc',
|
||||
isShallow: true, hasMergeBase: false,
|
||||
}), 0)
|
||||
})
|
||||
|
||||
test('shallow checkout WITH a merge-base keeps the exact count (reliable)', () => {
|
||||
assert.equal(resolveBehindCount({
|
||||
countStr: '3', currentSha: 'aaa', targetSha: 'bbb',
|
||||
isShallow: true, hasMergeBase: true,
|
||||
}), 3)
|
||||
})
|
||||
|
||||
test('full (non-shallow) clone keeps the exact count path unchanged', () => {
|
||||
assert.equal(resolveBehindCount({
|
||||
countStr: '7', currentSha: 'aaa', targetSha: 'bbb',
|
||||
isShallow: false, hasMergeBase: true,
|
||||
}), 7)
|
||||
})
|
||||
|
||||
test('up-to-date full clone reports 0', () => {
|
||||
assert.equal(resolveBehindCount({
|
||||
countStr: '0', currentSha: 'x', targetSha: 'x',
|
||||
isShallow: false, hasMergeBase: true,
|
||||
}), 0)
|
||||
})
|
||||
|
||||
test('non-numeric count falls back to 0 (defensive, unchanged behaviour)', () => {
|
||||
assert.equal(resolveBehindCount({
|
||||
countStr: '', currentSha: 'aaa', targetSha: 'bbb',
|
||||
isShallow: false, hasMergeBase: true,
|
||||
}), 0)
|
||||
})
|
||||
|
|
@ -37,7 +37,7 @@
|
|||
"test:desktop:nsis": "node scripts/test-desktop.mjs nsis",
|
||||
"test:desktop:existing": "node scripts/test-desktop.mjs existing",
|
||||
"test:desktop:fresh": "node scripts/test-desktop.mjs fresh",
|
||||
"test:desktop:platforms": "node --test electron/bootstrap-platform.test.cjs electron/hardening.test.cjs electron/backend-env.test.cjs electron/backend-probes.test.cjs electron/backend-ready.test.cjs electron/bootstrap-runner.test.cjs electron/connection-config.test.cjs electron/dashboard-token.test.cjs electron/gateway-ws-probe.test.cjs electron/oauth-net-request.test.cjs electron/desktop-uninstall.test.cjs electron/session-windows.test.cjs electron/link-title-window.test.cjs electron/workspace-cwd.test.cjs electron/fs-read-dir.test.cjs electron/git-root.test.cjs electron/windows-child-process.test.cjs electron/update-remote.test.cjs electron/update-rebuild.test.cjs electron/update-marker.test.cjs electron/update-relaunch.test.cjs electron/windows-user-env.test.cjs electron/window-state.test.cjs",
|
||||
"test:desktop:platforms": "node --test electron/bootstrap-platform.test.cjs electron/hardening.test.cjs electron/backend-env.test.cjs electron/backend-probes.test.cjs electron/backend-ready.test.cjs electron/bootstrap-runner.test.cjs electron/connection-config.test.cjs electron/dashboard-token.test.cjs electron/gateway-ws-probe.test.cjs electron/oauth-net-request.test.cjs electron/desktop-uninstall.test.cjs electron/session-windows.test.cjs electron/link-title-window.test.cjs electron/workspace-cwd.test.cjs electron/fs-read-dir.test.cjs electron/git-root.test.cjs electron/windows-child-process.test.cjs electron/update-remote.test.cjs electron/update-count.test.cjs electron/update-rebuild.test.cjs electron/update-marker.test.cjs electron/update-relaunch.test.cjs electron/windows-user-env.test.cjs electron/window-state.test.cjs",
|
||||
"typecheck": "tsc -p . --noEmit",
|
||||
"lint": "eslint src/ electron/",
|
||||
"lint:fix": "eslint src/ electron/ --fix",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue