Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions src/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,23 @@ get_script_dir () {
this.config = await Config.load({root: join(this.clientRoot, version)})
}

// Resolves the absolute path of the active install directory. Installed
// directories are named "<version>-<sha>", but `this.config.version` is plain
// semver, so match by exact name or "<version>-" prefix. Falls back to the
// bare version path when no match exists.
private async resolveActiveDir(): Promise<string> {
const {version} = this.config
try {
const entries = await readdir(this.clientRoot)
const match = entries.find((entry) => entry === version || entry.startsWith(`${version}-`))
if (match) return join(this.clientRoot, match)
} catch {
// fall through to bare version path
}

return join(this.clientRoot, version)
}

// removes any unused CLIs
private async tidy(): Promise<void> {
debug('tidy')
Expand All @@ -209,8 +226,15 @@ get_script_dir () {
if (!existsSync(root)) return
const files = await ls(root)

const isNotSpecial = (fPath: string, version: string): boolean =>
!['bin', 'current', version].includes(basename(fPath))
// `version` is plain semver (e.g. "1.2.3") but installed directories are
// named "<version>-<sha>" (e.g. "1.2.3-abc1234"). Protect both forms so
// the active CLI never deletes itself. See #1361.
const isNotSpecial = (fPath: string, version: string): boolean => {
const name = basename(fPath)
if (name === 'bin' || name === 'current') return false
// "1.2.3" or "1.2.3-abc1234" both start from `version`
return !(name === version || name.startsWith(`${version}-`))
}

const isOld = (fStat: Stats): boolean => {
const {mtime} = fStat
Expand All @@ -231,7 +255,12 @@ get_script_dir () {
private async touch(): Promise<void> {
// touch the client so it won't be tidied up right away
try {
const p = join(this.clientRoot, this.config.version)
// `this.config.version` is plain semver, but the installed directory is
// named "<version>-<sha>", so joining the bare version would point at a
// non-existent path and silently no-op. Resolve the real directory the
// same way tidy()'s guard does so the active install's mtime is refreshed
// and it isn't considered "old". See #1361.
const p = await this.resolveActiveDir()
debug('touching client at', p)
if (!existsSync(p)) return
return utimes(p, new Date(), new Date())
Expand Down
100 changes: 99 additions & 1 deletion test/update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {expect} from 'chai'
import {got} from 'got'
import nock from 'nock'
import {existsSync} from 'node:fs'
import {mkdir, rm, symlink, utimes, writeFile} from 'node:fs/promises'
import {mkdir, rm, stat, symlink, utimes, writeFile} from 'node:fs/promises'
import path from 'node:path'
import zlib from 'node:zlib'
import sinon from 'sinon'
Expand Down Expand Up @@ -69,6 +69,29 @@ const setupTidyClientRoot = async (config: Interfaces.Config): Promise<string> =
return root
}

// Mirrors a real install where the active directory is named "<version>-<sha>"
// (e.g. "2.0.0-a2559bd") while config.version remains plain semver ("2.0.0").
const setupShaSuffixedTidyClientRoot = async (config: Interfaces.Config, activeDirName: string): Promise<string> => {
const root = config.scopedEnvVar('OCLIF_CLIENT_HOME') || path.join(config.dataDir, 'client')
await mkdir(root, {recursive: true})

// Active version directory, named "<version>-<sha>"
const versionDir = path.join(root, activeDirName)
await mkdir(path.join(versionDir, 'bin'), {recursive: true})
await writeFile(path.join(versionDir, 'bin', config.bin), 'binary', 'utf8')

// bin/ directory with launcher script pointing at the sha-suffixed dir
await mkdir(path.join(root, 'bin'), {recursive: true})
await writeFile(path.join(root, 'bin', config.bin), `../${activeDirName}/bin`, 'utf8')

// current symlink -> sha-suffixed dir
if (!existsSync(path.join(root, 'current'))) {
await symlink(path.join(root, activeDirName), path.join(root, 'current'))
}

return root
}

describe('update plugin', () => {
let config: Config
let updater: Updater
Expand Down Expand Up @@ -388,5 +411,80 @@ describe('update plugin', () => {
// Recent version should survive
expect(existsSync(recentVersionDir)).to.be.true
})

// Regression test for #1361: installs are named "<version>-<sha>" but
// config.version is plain semver, so the exact-match guard never matched the
// active dir. Once it aged past 42 days, tidy() deleted the running CLI,
// leaving `current` dangling. The active dir must be preserved by name even
// when it is sha-suffixed and old.
it('should preserve a sha-suffixed active version directory even when old', async () => {
const activeDirName = '2.0.0-a2559bd'
clientRoot = await setupShaSuffixedTidyClientRoot(config, activeDirName)

// Create an old, non-active version that should be cleaned up
const oldVersionDir = path.join(clientRoot, '1.0.0-abc1234')
await mkdir(path.join(oldVersionDir, 'bin'), {recursive: true})
await writeFile(path.join(oldVersionDir, 'bin', 'example-cli'), 'old version', 'utf8')
await setOldMtime(oldVersionDir)

// Backdate bin/, current, and the active dir past the 42-day threshold to
// exercise the exact scenario from the report: staying on latest 42+ days.
await setOldMtime(path.join(clientRoot, 'bin'))
await setOldMtime(path.join(clientRoot, 'current'))
await setOldMtime(path.join(clientRoot, activeDirName))

// Manifest carries a sha so `updated` becomes "2.0.0-a2559bd", matching
// the current install and short-circuiting before any download, while
// touch() and tidy() still run.
const manifestRegex = new RegExp(
`channels\\/stable\\/example-cli-${config.platform}-${config.arch}-buildmanifest`,
)
nock(/oclif-staging.s3.amazonaws.com/)
.get(manifestRegex)
.reply(200, {sha: 'a2559bd', version: '2.0.0'})

updater = initUpdater(config)
await updater.runUpdate({autoUpdate: false})

// Active sha-suffixed dir must survive, along with bin/ and current.
expect(existsSync(path.join(clientRoot, activeDirName))).to.be.true
expect(existsSync(path.join(clientRoot, 'bin'))).to.be.true
expect(existsSync(path.join(clientRoot, 'current'))).to.be.true

// The `current` symlink must still resolve to the active dir (not dangling).
expect(existsSync(path.join(clientRoot, 'current', 'bin', config.bin))).to.be.true

// Genuinely old, non-active version is still cleaned up.
expect(existsSync(oldVersionDir)).to.be.false
})

// touch() must refresh the sha-suffixed active dir's mtime so it is never
// considered "old" in the first place. Previously it joined the bare
// config.version ("2.0.0"), which does not exist on disk, making it a no-op.
it('touch() refreshes the sha-suffixed active version directory mtime', async () => {
const activeDirName = '2.0.0-a2559bd'
clientRoot = await setupShaSuffixedTidyClientRoot(config, activeDirName)

const activeDir = path.join(clientRoot, activeDirName)

// Backdate the active dir well past the threshold.
await setOldMtime(activeDir)
const beforeMtime = (await stat(activeDir)).mtime.getTime()

const manifestRegex = new RegExp(
`channels\\/stable\\/example-cli-${config.platform}-${config.arch}-buildmanifest`,
)
nock(/oclif-staging.s3.amazonaws.com/)
.get(manifestRegex)
.reply(200, {sha: 'a2559bd', version: '2.0.0'})

updater = initUpdater(config)
await updater.runUpdate({autoUpdate: false})

// mtime should have been bumped to ~now, proving touch() found the dir.
const afterMtime = (await stat(activeDir)).mtime.getTime()
expect(afterMtime).to.be.greaterThan(beforeMtime)
expect(Date.now() - afterMtime).to.be.lessThan(60_000)
})
})
})
Loading