Skip to content

tidy() deletes the active version directory for sha-suffixed installs (<version>-<sha>), breaking the CLI #1361

Description

@khaled4vokalz

Generated using OpenCode and Claude, but validated locally.

Describe the bug

tidy() in src/update.ts deletes the active/running version directory for any install whose directory is named <version>-<sha> (e.g. 2.0.9-a2559bd), once that version has been the active one for 42+ days. It leaves bin/ and current in place, so the current symlink becomes dangling and the CLI is broken (command not found / ENOENT).
The root cause is a name-matching mismatch that plugin-update creates itself:

  • plugin-update names the extracted dir ${manifest.version}-${manifest.sha} (src/update.ts:112 and :124), so on disk you get 2.0.9-a2559bd.
  • But this.config.version is read from that dir's package.json "version", which is plain semver (2.0.9).
    tidy()'s name guard uses exact-match:
// src/update.ts:212-213
const isNotSpecial = (fPath, version) =>
  !['bin', 'current', version].includes(basename(fPath))

['bin','current','2.0.9'].includes('2.0.9-a2559bd')falseisNotSpecial returns true → the active dir is not protected by name.
The age backstop also fails, because touch() targets the wrong path:

// src/update.ts:234
const p = join(this.clientRoot, this.config.version) // -> .../client/2.0.9 (does NOT exist)
if (!existsSync(p)) return                            // silent no-op

The real dir is .../client/2.0.9-a2559bd, so existsSync is false and touch() never refreshes the active dir's mtime. The dir keeps its original extraction mtime and ages past the 42-day threshold.
With both guards failing for the active dir, tidy() selects it for rm -rf while keeping bin/ and current.

Regression history

This used to be handled. ca57247 (#621, "clis don't delete themselves") added an explicit if (basename(f.path).startsWith(this.config.version)) return guard, commented // if 1.2.3-shasha7 starts with 1.2.3. The ESM refactor be87bf6 (#643) rewrote tidy() and dropped the startsWith semantics, collapsing it into the exact-match includes() above (and also introduced an args-swap bug). #1290 later fixed only the args swap (restoring bin/current protection) but did not restore startsWith, so sha-suffixed active installs remain unprotected.

To Reproduce

In the real world: install a CLI whose manifests carry a sha (so dirs are named <version>-<sha>), stay on the latest version for 42+ days, then run <cli> update (or let autoupdate run while you're already on the latest). tidy() deletes the active dir and the CLI breaks.
To prove the exact selection logic deterministically without waiting 42 days or bricking an install, the following self-contained script replicates tidy()'s filter (isNotSpecial(f.path, this.config.version) && isOld(f.stat)) and touch() against a throwaway fixture:

  1. Save as repro.mjs:
import {mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync, existsSync} from 'node:fs'
import {readdir, stat, utimes} from 'node:fs/promises'
import {basename, join} from 'node:path'
import {tmpdir} from 'node:os'

const root = mkdtempSync(join(tmpdir(), 'client-'))
const ACTIVE = '2.0.9-a2559bd'   // active dir, sha-suffixed (as plugin-update names it)
const OLD = '2.0.7-13e82f8'      // genuinely old, non-active
const CONFIG_VERSION = '2.0.9'   // this.config.version (plain semver from package.json)

mkdirSync(join(root, 'bin'))
writeFileSync(join(root, 'bin', 'cli'), '')
mkdirSync(join(root, ACTIVE))
mkdirSync(join(root, OLD))
symlinkSync(`./${ACTIVE}`, join(root, 'current'))

const now = Date.now(), day = 86_400_000
const old50 = new Date(now - 50 * day) // 50 days old => exceeds 42-day threshold
for (const d of [ACTIVE, OLD, 'bin']) await utimes(join(root, d), old50, old50)

// touch() backstop: join(clientRoot, config.version) -> '2.0.9' (no sha)
const p = join(root, CONFIG_VERSION)
console.log(`touch() target: ${basename(p)} exists? ${existsSync(p)} -> ${existsSync(p) ? 'refreshed' : 'NO-OP'}\n`)
if (existsSync(p)) await utimes(p, new Date(), new Date())

// ls() uses stat() (follows symlinks), as in src/util.ts
const ls = async (dir) => Promise.all((await readdir(dir)).map(f => {
  const path = join(dir, f); return stat(path).then(s => ({path, stat: s}))
}))
const isNotSpecial = (fp, v) => !['bin', 'current', v].includes(basename(fp))
const isOld = (s) => { const m = new Date(s.mtime); m.setHours(m.getHours() + 42*24); return m < new Date() }

const selected = []
for (const f of await ls(root)) {
  const del = isNotSpecial(f.path, CONFIG_VERSION) && isOld(f.stat)
  if (del) selected.push(basename(f.path))
  console.log(`${basename(f.path).padEnd(20)} ${del ? '*** rm -rf ***' : 'kept'}`)
}
console.log('\nSelected for deletion:', selected.join(', '))
console.log('Active dir deleted?', selected.includes(ACTIVE) ? 'YES (BUG)' : 'no')
rmSync(root, {recursive: true, force: true})
  1. Run it:
node repro.mjs
  1. See output:
touch() target: 2.0.9 exists? false -> NO-OP
2.0.7-13e82f8        *** rm -rf ***
2.0.9-a2559bd        *** rm -rf ***
bin                  kept
current              kept
Selected for deletion: 2.0.7-13e82f8, 2.0.9-a2559bd
Active dir deleted? YES (BUG)

The active dir 2.0.9-a2559bd is selected for deletion while bin/current survive → current becomes a dangling symlink and the CLI is broken.

Expected behavior
tidy() should never delete the active/running version directory. It should protect both <version> and <version>-<sha> directories (as it did before #643), and touch() should refresh the actual installed directory's mtime even when it is sha-suffixed. Only genuinely unused old versions should be removed.

Screenshots
N/A (CLI output included above).

Environment (please complete the following information):

Additional context
Likely minimal fix (two coordinated changes in src/update.ts):

  1. Restore startsWith in the name guard (src/update.ts:212-213):
const isNotSpecial = (fPath, version) => {
  const name = basename(fPath)
  if (name === 'bin' || name === 'current') return false
  return !(name === version || name.startsWith(`${version}-`)) // protects <version> and <version>-<sha>
}
  1. Make touch() target the real dir (src/update.ts:234) by resolving the <version>-<sha> entry instead of bare config.version:
const dirs = await readdir(this.clientRoot)
const activeDir = dirs.find(d => d === this.config.version || d.startsWith(`${this.config.version}-`))
const p = join(this.clientRoot, activeDir ?? this.config.version)

Either change alone closes the hole; doing both restores the pre-#643 behavior and re-enables the age backstop for sha-suffixed installs. A regression test should use a fixture dir named <version>-<sha> with a backdated mtime and assert the active dir survives tidy() while a separate old non-active version is removed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions