hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 28 Packages Projects Releases Wiki Activity

fix(desktop): make shallow update status presence-only

Browse Source
This commit is contained in:
metamindedu
2026-07-14 23:47:33 +09:00
committed by Teknium
parent 54bd47b811
commit 3dbdea8b8b
3 changed files with 195 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
apps/desktop/electron/main.ts
View File

@@ -206,7 +206,7 @@ import {
} from './ssh-connection'
import { createStreamThrottle } from './stream-throttle'
import { nativeOverlayWidth as computeNativeOverlayWidth, macTitleBarOverlayHeight } from './titlebar-overlay-width'
import { resolveBehindCount, shouldCountCommits } from './update-count'
import { resolveBehindCount, resolveCommitLogSelection, shouldCountCommits } from './update-count'
import { waitForUpdateClearance } from './update-gate'
import { readLiveUpdateMarker, updateHandoffConflict, writeUpdateMarker } from './update-marker'
import { isOfficialSshRemote, OFFICIAL_REPO_HTTPS_URL } from './update-remote'
@@ -2603,40 +2603,40 @@ async function checkUpdates() {
const git = args => runGit(args, { cwd: updateRoot }).then(r => r.stdout.trim())
const [currentSha, targetSha, dirtyStr, currentBranch, shallowStr, mergeBaseStr] = await Promise.all([
const [currentSha, targetSha, dirtyStr, currentBranch, shallowStr] = await Promise.all([
git(['rev-parse', 'HEAD']),
git(['rev-parse', `origin/${branch}`]),
git(['status', '--porcelain']),
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}`])
git(['rev-parse', '--is-shallow-repository'])
])
const isShallow = shallowStr === 'true'
const hasMergeBase = Boolean(mergeBaseStr)
// Only enumerate the commit count when it is meaningful. On a shallow checkout
// with no merge-base, `rev-list --count` walks the entire remote ancestry
// (thousands of commits, see #51922) and resolveBehindCount discards the
// result anyway in favour of a SHA compare — so skip the expensive query.
const countStr = shouldCountCommits({ isShallow, hasMergeBase })
? await git(['rev-list', `HEAD..origin/${branch}`, '--count'])
: ''
// A shallow graph cannot provide a trustworthy exact count, even when it has
// a visible merge-base. Skip the ancestry walk and use the SHA fallback.
const countStr = shouldCountCommits({ isShallow }) ? await git(['rev-list', `HEAD..origin/${branch}`, '--count']) : ''
// A positive directional ancestry result remains trustworthy in a shallow
// graph and prevents a local commit on top of origin from looking outdated.
const targetIsAncestorOfHead =
isShallow &&
currentSha !== targetSha &&
(await runGit(['merge-base', '--is-ancestor', `origin/${branch}`, 'HEAD'], { cwd: updateRoot })).code === 0
const behind = resolveBehindCount({
countStr,
currentSha,
targetSha,
isShallow,
hasMergeBase
targetIsAncestorOfHead
})
// behind === null means "update available, exact count unknown" (shallow
// clone without a merge-base): still list what origin offers (the log read
// is capped at 40 entries), so "See what's new" stays useful and honest.
const commits = behind !== 0 ? await readCommitLog(updateRoot, branch) : []
// clone): still list what origin offers — resolveCommitLogSelection keeps
// the shallow log to the fetched tip so the range walk can't enumerate the
// contaminated ancestry — so "See what's new" stays useful and honest.
const commits = behind !== 0 ? await readCommitLog(updateRoot, branch, isShallow) : []
return {
supported: true,
@@ -2653,12 +2653,13 @@ async function checkUpdates() {
}
}
async function readCommitLog(cwd, branch) {
async function readCommitLog(cwd, branch, isShallow) {
const SEP = '\x1f'
const REC = '\x1e'
const { limit, revision } = resolveCommitLogSelection({ branch, isShallow })
const { stdout } = await runGit(
['log', `HEAD..origin/${branch}`, `--pretty=format:%H${SEP}%s${SEP}%an${SEP}%at${REC}`, '-n', '40'],
['log', revision, `--pretty=format:%H${SEP}%s${SEP}%an${SEP}%at${REC}`, '-n', String(limit)],
{ cwd }
)

184
apps/desktop/electron/update-count.test.ts
View File

@@ -1,21 +1,45 @@
import assert from 'node:assert/strict'
import { execFileSync } from 'node:child_process'
import fs from 'node:fs'
import os from 'node:os'
import path from 'node:path'
import { test } from 'vitest'
import { resolveBehindCount, shouldCountCommits } from './update-count'
import { resolveBehindCount, resolveCommitLogSelection, shouldCountCommits } from './update-count'
// FAIL-BEFORE: the shallow/no-merge-base branch returned the sentinel `1`,
// which the UI rendered as a literal "1 change included" even when the true
// count was far higher (e.g. 90). An update IS available here, but its exact
// size is unknown — the only honest value is `null`.
function createTempGitRepo() {
const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-update-count-'))
const git = (...args: string[]) => execFileSync('git', args, { cwd, encoding: 'utf8', timeout: 10_000 }).trim()
try {
git('init', '--quiet')
git('config', 'commit.gpgSign', 'false')
git('config', 'core.hooksPath', '.git/no-hooks')
git('config', 'user.name', 'Hermes Test')
git('config', 'user.email', 'hermes@example.invalid')
return { cwd, git }
} catch (error) {
fs.rmSync(cwd, { recursive: true, force: true })
throw error
}
}
// 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) — #51922. Later the branch returned the sentinel
// `1`, which the UI rendered as a literal "1 change included" even when the
// true count was far higher (e.g. 90, or the real-world 61 in #84591). An
// update IS available here, but its exact size is unknown — the only honest
// value is `null`.
test('shallow checkout with no merge-base reports null (unknown count), not a fake 1', () => {
assert.equal(
resolveBehindCount({
countStr: '12104',
currentSha: 'aaa',
targetSha: 'bbb',
isShallow: true,
hasMergeBase: false
isShallow: true
}),
null
)
@@ -27,23 +51,114 @@ test('shallow checkout with no merge-base but identical SHA reports up-to-date',
countStr: '12104',
currentSha: 'abc',
targetSha: 'abc',
isShallow: true,
hasMergeBase: false
isShallow: true
}),
0
)
})
test('shallow checkout WITH a merge-base keeps the exact count (reliable)', () => {
test('shallow local-ahead checkout reports up-to-date when origin is a known ancestor', () => {
assert.equal(
resolveBehindCount({
countStr: '',
currentSha: 'local-child',
targetSha: 'origin-parent',
isShallow: true,
targetIsAncestorOfHead: true
}),
0
)
})
test('shallow Git graph proves the remote tip is an ancestor of a local commit', () => {
const { cwd, git } = createTempGitRepo()
try {
git('commit', '--allow-empty', '-m', 'origin tip')
const targetSha = git('rev-parse', 'HEAD')
git('update-ref', 'refs/remotes/origin/main', targetSha)
fs.writeFileSync(path.join(cwd, '.git', 'shallow'), `${targetSha}\n`)
git('commit', '--allow-empty', '-m', 'local child')
const currentSha = git('rev-parse', 'HEAD')
git('merge-base', '--is-ancestor', 'origin/main', 'HEAD')
assert.notEqual(currentSha, targetSha)
assert.equal(
resolveBehindCount({
countStr: '',
currentSha,
targetSha,
isShallow: true,
targetIsAncestorOfHead: true
}),
0
)
} finally {
fs.rmSync(cwd, { recursive: true, force: true })
}
}, 30_000)
test('shallow checkout with a merge-base does not trust an inflated rev-list count', () => {
const { cwd, git } = createTempGitRepo()
try {
git('commit', '--allow-empty', '-m', 'root')
git('commit', '--allow-empty', '-m', 'ancestor')
const redundantParent = git('rev-parse', 'HEAD')
git('commit', '--allow-empty', '-m', 'installed head')
const currentSha = git('rev-parse', 'HEAD')
const tree = git('rev-parse', 'HEAD^{tree}')
const targetSha = execFileSync('git', ['commit-tree', tree, '-p', currentSha, '-p', redundantParent], {
cwd,
encoding: 'utf8',
input: 'remote merge\n',
timeout: 10_000
}).trim()
git('update-ref', 'refs/remotes/origin/main', targetSha)
const completeCount = git('rev-list', 'HEAD..origin/main', '--count')
assert.equal(completeCount, '1')
fs.writeFileSync(path.join(cwd, '.git', 'shallow'), `${currentSha}\n`)
assert.equal(git('rev-parse', '--is-shallow-repository'), 'true')
assert.equal(git('merge-base', 'HEAD', 'origin/main'), currentSha)
const shallowCount = git('rev-list', 'HEAD..origin/main', '--count')
assert.ok(Number.parseInt(shallowCount, 10) > Number.parseInt(completeCount, 10))
assert.equal(
resolveBehindCount({
countStr: shallowCount,
currentSha,
targetSha,
isShallow: true
}),
null
)
} finally {
fs.rmSync(cwd, { recursive: true, force: true })
}
}, 30_000)
test('shallow checkout with a merge-base still uses presence-only status', () => {
assert.equal(
resolveBehindCount({
countStr: '3',
currentSha: 'aaa',
targetSha: 'bbb',
isShallow: true,
hasMergeBase: true
isShallow: true
}),
3
null
)
})
@@ -53,8 +168,7 @@ test('full (non-shallow) clone keeps the exact count path unchanged', () => {
countStr: '7',
currentSha: 'aaa',
targetSha: 'bbb',
isShallow: false,
hasMergeBase: true
isShallow: false
}),
7
)
@@ -66,8 +180,7 @@ test('up-to-date full clone reports 0', () => {
countStr: '0',
currentSha: 'x',
targetSha: 'x',
isShallow: false,
hasMergeBase: true
isShallow: false
}),
0
)
@@ -79,28 +192,35 @@ test('non-numeric count falls back to 0 (defensive, unchanged behaviour)', () =>
countStr: '',
currentSha: 'aaa',
targetSha: 'bbb',
isShallow: false,
hasMergeBase: true
isShallow: false
}),
0
)
})
// shouldCountCommits gates the expensive `rev-list --count` in checkUpdates().
// FAIL-BEFORE: in the shallow + no-merge-base case the caller ran rev-list
// unconditionally and discarded the bogus result; this predicate lets the
// caller SKIP the whole-ancestry enumeration in exactly that case (#51922).
test('shallow checkout with no merge-base SKIPS the rev-list count', () => {
assert.equal(shouldCountCommits({ isShallow: true, hasMergeBase: false }), false)
// Every shallow graph is incomplete, so a visible merge-base is not enough to
// prove that the count is exact.
test('shallow checkouts skip the rev-list count', () => {
assert.equal(shouldCountCommits({ isShallow: true }), false)
})
test('shallow checkout WITH a merge-base still runs the count', () => {
assert.equal(shouldCountCommits({ isShallow: true, hasMergeBase: true }), true)
test('full (non-shallow) clones run the rev-list count', () => {
assert.equal(shouldCountCommits({ isShallow: false }), true)
})
test('full (non-shallow) clone always runs the count', () => {
assert.equal(shouldCountCommits({ isShallow: false, hasMergeBase: true }), true)
assert.equal(shouldCountCommits({ isShallow: false, hasMergeBase: false }), true)
test('shallow commit logs select only the fetched remote tip', () => {
assert.deepEqual(resolveCommitLogSelection({ branch: 'main', isShallow: true }), {
limit: 1,
revision: 'origin/main'
})
})
test('full-clone commit logs keep the complete behind range', () => {
assert.deepEqual(resolveCommitLogSelection({ branch: 'release', isShallow: false }), {
limit: 40,
revision: 'HEAD..origin/release'
})
})
// The skip path produces an empty countStr; resolveBehindCount must NOT trust
@@ -111,8 +231,7 @@ test('skipped-count path resolves via SHA compare, never via empty countStr', ()
countStr: '',
currentSha: 'aaa',
targetSha: 'bbb',
isShallow: true,
hasMergeBase: false
isShallow: true
}),
null
)
@@ -121,8 +240,7 @@ test('skipped-count path resolves via SHA compare, never via empty countStr', ()
countStr: '',
currentSha: 'same',
targetSha: 'same',
isShallow: true,
hasMergeBase: false
isShallow: true
}),
0
)

40
apps/desktop/electron/update-count.ts
View File

@@ -1,23 +1,20 @@
// Whether `git rev-list HEAD..origin/<branch> --count` produces a meaningful
// number worth computing. On a SHALLOW checkout (installer clones with
// --depth 1) the local history often shares no merge-base with the freshly
// fetched origin tip, so the count enumerates the entire remote ancestry and
// returns a bogus huge number (e.g. 12104) — see #51922. resolveBehindCount
// discards that bogus count in favour of a SHA compare, so the caller should
// SKIP the expensive rev-list entirely in that case rather than run it and
// throw the result away.
function shouldCountCommits({ isShallow, hasMergeBase }) {
return !(isShallow && !hasMergeBase)
// number worth computing. Installer checkouts are shallow (`--depth 1`), so
// their visible graph is incomplete even when `merge-base` happens to find a
// common commit. A merge can expose ancestry that the local shallow boundary
// hides from HEAD, inflating the count with old commits. Exact counts are only
// trustworthy in full clones; shallow checkouts use presence-only status plus
// any positively proven local-ahead ancestry.
function shouldCountCommits({ isShallow }) {
return !isShallow
}
// Resolve how many commits the local checkout is behind origin for the desktop
// update indicator. When the count isn't meaningful (shallow + no merge-base)
// 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 (!shouldCountCommits({ isShallow, hasMergeBase })) {
if (currentSha && targetSha && currentSha === targetSha) {
// update indicator. Shallow checkouts use SHA equality plus any positively
// proven local-ahead ancestry; exact counts remain exclusive to full clones.
function resolveBehindCount({ countStr, currentSha, targetSha, isShallow, targetIsAncestorOfHead = false }) {
if (!shouldCountCommits({ isShallow })) {
if (currentSha && targetSha && (currentSha === targetSha || targetIsAncestorOfHead)) {
return 0
}
@@ -31,4 +28,13 @@ function resolveBehindCount({ countStr, currentSha, targetSha, isShallow, hasMer
return Number.parseInt(countStr, 10) || 0
}
export { resolveBehindCount, shouldCountCommits }
// Shallow history can also contaminate the changelog range. Trust the fetched
// remote tip itself, but do not walk its ancestry. Full clones retain the
// detailed range used by the existing update overlay.
function resolveCommitLogSelection({ branch, isShallow }) {
const remote = `origin/${branch}`
return isShallow ? { limit: 1, revision: remote } : { limit: 40, revision: `HEAD..${remote}` }
}
export { resolveBehindCount, resolveCommitLogSelection, shouldCountCommits }