From 3190b30e970c7495ed3ae9cfae3ea2fb30ee0834 Mon Sep 17 00:00:00 2001
From: unkin-agent <unkin-agent@unkin.net>
Date: Tue, 8 Sep 2026 23:32:04 +1000
Subject: [PATCH] Abort clean when a plugin spec fails to parse

An unparseable entry dropped out of the wanted set, so clean deleted the
directory and lock entry of a plugin the user still has configured.

Add tests/init_spec.lua covering setup, sync, clean and list.
---
 lua/arti-pack/init.lua |  11 +-
 tests/init_spec.lua    | 407 +++++++++++++++++++++++++++++++++++++++++
 tests/run.lua          |   2 +-
 3 files changed, 418 insertions(+), 2 deletions(-)
 create mode 100644 tests/init_spec.lua

diff --git a/lua/arti-pack/init.lua b/lua/arti-pack/init.lua
index 4b09a78..c82b6c9 100644
--- a/lua/arti-pack/init.lua
+++ b/lua/arti-pack/init.lua
@@ -119,7 +119,16 @@ function M.sync(opts)
 end
 
 function M.clean()
-  local specs = spec_mod.parse_all(M.config.plugins)
+  local specs, errors = spec_mod.parse_all(M.config.plugins)
+  for _, err in ipairs(errors) do
+    notify(err, vim.log.levels.ERROR)
+  end
+  -- An unparseable entry still names a plugin the user wants, so the set of
+  -- wanted names is incomplete; deleting from it would remove a live install.
+  if #errors > 0 then
+    return notify('clean aborted, no plugin spec may be in error', vim.log.levels.WARN)
+  end
+
   local wanted = {}
   for _, spec in ipairs(specs) do
     wanted[spec.name] = true
diff --git a/tests/init_spec.lua b/tests/init_spec.lua
new file mode 100644
index 0000000..068bf02
--- /dev/null
+++ b/tests/init_spec.lua
@@ -0,0 +1,407 @@
+local t = require('tests.harness')
+local fs = require('arti-pack.fs')
+local Lock = require('arti-pack.lock')
+local arti = require('arti-pack')
+
+local function tmpdir()
+  local dir = vim.fn.tempname()
+  vim.fn.mkdir(dir, 'p')
+  return dir
+end
+
+local function write(path, contents)
+  local fd = assert(io.open(path, 'wb'))
+  fd:write(contents)
+  fd:close()
+  return path
+end
+
+local function have(cmd)
+  return vim.fn.executable(cmd) == 1
+end
+
+local env
+
+-- Point stdpath at a throwaway tree so each test gets its own pack dir, cache
+-- and lockfile, and collect notifications instead of printing them.
+local function use_env()
+  local base = tmpdir()
+  env = {
+    data = fs.mkdirp(base .. '/data'),
+    cache = fs.mkdirp(base .. '/cache'),
+    notices = {},
+  }
+  env.pack_dir = env.data .. '/site/pack/arti-pack'
+  env.lock_path = env.data .. '/arti-pack/lock.json'
+  return env
+end
+
+use_env()
+
+local real_stdpath = vim.fn.stdpath
+local real_notify = vim.notify
+
+vim.fn.stdpath = function(what)
+  if what == 'data' then
+    return env.data
+  elseif what == 'cache' then
+    return env.cache
+  end
+  return real_stdpath(what)
+end
+
+vim.notify = function(msg, level)
+  table.insert(env.notices, { msg = msg, level = level })
+end
+
+local function notices()
+  local out = {}
+  for _, n in ipairs(env.notices) do
+    table.insert(out, n.msg)
+  end
+  return table.concat(out, '\n')
+end
+
+local function last_notice()
+  return env.notices[#env.notices] and env.notices[#env.notices].msg or ''
+end
+
+local function lock()
+  return Lock.new(env.lock_path):load()
+end
+
+local function record(name, tag, opt)
+  local l = lock()
+  l:set(name, { tag = tag or 'v1', opt = opt == true })
+  assert(l:save())
+end
+
+local function place_dir(name, sub)
+  local dir = fs.mkdirp(env.pack_dir .. '/' .. sub .. '/' .. name)
+  write(dir .. '/marker.lua', 'return true')
+  return dir
+end
+
+local function install_fake(name, sub, tag)
+  place_dir(name, sub)
+  record(name, tag, sub == 'opt')
+end
+
+-- sync finishes asynchronously; its closing notification is the only signal.
+local function is_terminal(msg)
+  return msg:find('everything up to date', 1, true) ~= nil
+    or msg:find('%d+ installed') ~= nil
+    or msg:find('%d+ updated') ~= nil
+    or msg:find('%d+ failed') ~= nil
+end
+
+local function drain(from)
+  assert(
+    vim.wait(30000, function()
+      for i = from + 1, #env.notices do
+        if is_terminal(env.notices[i].msg) then
+          return true
+        end
+      end
+      return false
+    end, 20),
+    'sync never finished: ' .. notices()
+  )
+end
+
+local function sync(opts)
+  local before = #env.notices
+  arti.sync(opts)
+  drain(before)
+end
+
+local function build_archive(ext, repo, version)
+  local work = tmpdir()
+  local root = string.format('%s-%s', repo, version)
+  vim.fn.mkdir(work .. '/' .. root .. '/lua', 'p')
+  write(work .. '/' .. root .. '/lua/marker.lua', 'return "' .. version .. '"')
+
+  local archive = string.format('%s/%s.%s', tmpdir(), root, ext)
+  local out = vim.system({ 'zip', '-q', '-r', archive, root }, { cwd = work, text = true }):wait()
+  assert(out.code == 0, 'failed to build fixture archive: ' .. tostring(out.stderr))
+  return archive
+end
+
+local function plugin_entry(source, tag, archive, extra)
+  return vim.tbl_extend('force', { source, tag = tag, url = 'file://' .. archive }, extra or {})
+end
+
+t.describe('arti-pack.setup', function()
+  t.it('creates the start and opt pack directories', function()
+    use_env()
+    arti.setup({ auto_sync = false })
+    t.truthy(fs.exists(env.pack_dir .. '/start'), 'start directory is created')
+    t.truthy(fs.exists(env.pack_dir .. '/opt'), 'opt directory is created')
+  end)
+
+  t.it('keeps the defaults for options the caller omits', function()
+    use_env()
+    arti.setup({ auto_sync = false })
+    t.eq('https://artifactapi.k8s.syd1.au.unkin.net/api/v1/remote/github', arti.config.registry)
+    t.eq(4, arti.config.concurrency)
+    t.eq(120, arti.config.timeout)
+  end)
+
+  t.it('applies the options the caller does set', function()
+    use_env()
+    arti.setup({ auto_sync = false, registry = 'https://example.test/gh', concurrency = 1 })
+    t.eq('https://example.test/gh', arti.config.registry)
+    t.eq(1, arti.config.concurrency)
+    t.eq(120, arti.config.timeout, 'untouched options keep their default')
+  end)
+
+  t.it('does not splice defaults into the caller plugin list', function()
+    use_env()
+    arti.setup({ auto_sync = false, plugins = { { 'owner/one', tag = 'v1' }, 'owner/two' } })
+    t.eq(2, #arti.config.plugins)
+    t.eq('owner/one', arti.config.plugins[1][1])
+    t.eq('owner/two', arti.config.plugins[2])
+  end)
+
+  t.it('forgets the plugin list of an earlier setup call', function()
+    use_env()
+    arti.setup({ auto_sync = false, plugins = { { 'owner/one', tag = 'v1' } } })
+    arti.setup({ auto_sync = false })
+    t.eq({}, arti.config.plugins)
+  end)
+
+  t.it('syncs on its own when auto_sync is left on', function()
+    use_env()
+    local before = #env.notices
+    arti.setup({ plugins = {} })
+    drain(before)
+    t.matches('everything up to date', notices())
+  end)
+end)
+
+t.describe('arti-pack.sync', function()
+  if not have('zip') or not have('unzip') then
+    t.it('SKIPPED: zip/unzip unavailable', function() end)
+    return
+  end
+
+  t.it('installs a configured plugin and records it in the lockfile', function()
+    use_env()
+    local archive = build_archive('zip', 'thing.nvim', '1.0.0')
+    arti.setup({ auto_sync = false, plugins = { plugin_entry('owner/thing.nvim', 'v1.0.0', archive) } })
+    sync()
+
+    t.truthy(fs.exists(env.pack_dir .. '/start/thing.nvim/lua/marker.lua'), 'plugin is unpacked into pack/start')
+    t.eq('v1.0.0', lock():get('thing.nvim').tag)
+  end)
+
+  t.it('installs an opt plugin under pack/opt', function()
+    use_env()
+    local archive = build_archive('zip', 'lazy.nvim', '1.0.0')
+    arti.setup({
+      auto_sync = false,
+      plugins = { plugin_entry('folke/lazy.nvim', 'v1.0.0', archive, { opt = true }) },
+    })
+    sync()
+
+    t.truthy(fs.exists(env.pack_dir .. '/opt/lazy.nvim/lua/marker.lua'))
+    t.falsy(fs.exists(env.pack_dir .. '/start/lazy.nvim'))
+  end)
+
+  t.it('does no work on a second run', function()
+    use_env()
+    local archive = build_archive('zip', 'thing.nvim', '1.0.0')
+    arti.setup({ auto_sync = false, plugins = { plugin_entry('owner/thing.nvim', 'v1.0.0', archive) } })
+    sync()
+
+    env.notices = {}
+    sync()
+    t.matches('everything up to date', last_notice())
+  end)
+
+  t.it('reinstalls a current plugin when forced', function()
+    use_env()
+    local archive = build_archive('zip', 'thing.nvim', '1.0.0')
+    arti.setup({ auto_sync = false, plugins = { plugin_entry('owner/thing.nvim', 'v1.0.0', archive) } })
+    sync()
+    fs.rm_rf(env.pack_dir .. '/start/thing.nvim')
+
+    env.notices = {}
+    sync({ force = true })
+    t.matches('1 updated', last_notice())
+    t.truthy(fs.exists(env.pack_dir .. '/start/thing.nvim/lua/marker.lua'))
+  end)
+
+  t.it('reinstalls a plugin whose directory disappeared', function()
+    use_env()
+    local archive = build_archive('zip', 'thing.nvim', '1.0.0')
+    arti.setup({ auto_sync = false, plugins = { plugin_entry('owner/thing.nvim', 'v1.0.0', archive) } })
+    sync()
+    fs.rm_rf(env.pack_dir .. '/start/thing.nvim')
+
+    sync()
+    t.truthy(fs.exists(env.pack_dir .. '/start/thing.nvim/lua/marker.lua'))
+  end)
+
+  t.it('reports a bad spec and still installs the good ones', function()
+    use_env()
+    local archive = build_archive('zip', 'thing.nvim', '1.0.0')
+    arti.setup({
+      auto_sync = false,
+      plugins = { plugin_entry('owner/thing.nvim', 'v1.0.0', archive), { 'not a source' } },
+    })
+    sync()
+
+    t.matches('plugins%[2%]', notices())
+    t.matches('invalid source', notices())
+    t.truthy(fs.exists(env.pack_dir .. '/start/thing.nvim/lua/marker.lua'))
+  end)
+
+  t.it('surfaces an install failure instead of recording the plugin', function()
+    use_env()
+    arti.setup({
+      auto_sync = false,
+      plugins = { plugin_entry('owner/thing.nvim', 'v1.0.0', tmpdir() .. '/absent.zip') },
+    })
+    sync()
+
+    t.matches('1 failed', last_notice())
+    t.falsy(lock():get('thing.nvim'), 'a failed install is not locked')
+  end)
+
+  t.it('refuses to start a second sync while one is running', function()
+    use_env()
+    local archive = build_archive('zip', 'thing.nvim', '1.0.0')
+    arti.setup({ auto_sync = false, plugins = { plugin_entry('owner/thing.nvim', 'v1.0.0', archive) } })
+
+    local before = #env.notices
+    arti.sync()
+    arti.sync()
+    t.matches('a sync is already running', last_notice())
+    drain(before)
+  end)
+end)
+
+t.describe('arti-pack.clean', function()
+  t.it('removes the directories and lock entries that are no longer configured', function()
+    use_env()
+    arti.setup({ auto_sync = false, plugins = { { 'owner/keep.nvim', tag = 'v1' } } })
+    install_fake('keep.nvim', 'start')
+    install_fake('drop.nvim', 'start')
+    install_fake('dropopt.nvim', 'opt')
+
+    arti.clean()
+
+    t.truthy(fs.exists(env.pack_dir .. '/start/keep.nvim'), 'a configured plugin is kept')
+    t.falsy(fs.exists(env.pack_dir .. '/start/drop.nvim'))
+    t.falsy(fs.exists(env.pack_dir .. '/opt/dropopt.nvim'))
+    t.eq({ 'keep.nvim' }, lock():names())
+    t.matches('removed drop.nvim, dropopt.nvim', last_notice())
+  end)
+
+  t.it('does nothing when every install is still configured', function()
+    use_env()
+    arti.setup({ auto_sync = false, plugins = { { 'owner/keep.nvim', tag = 'v1' } } })
+    install_fake('keep.nvim', 'start')
+
+    arti.clean()
+
+    t.matches('nothing to clean', last_notice())
+    t.truthy(fs.exists(env.pack_dir .. '/start/keep.nvim'))
+    t.eq({ 'keep.nvim' }, lock():names())
+  end)
+
+  t.it('drops a lock entry whose directory is already gone', function()
+    use_env()
+    arti.setup({ auto_sync = false, plugins = {} })
+    record('ghost.nvim', 'v1')
+
+    arti.clean()
+
+    t.eq({}, lock():names())
+  end)
+
+  -- Regression: an unparseable entry used to vanish from the wanted set, so
+  -- clean deleted a plugin the user still had configured.
+  t.it('deletes nothing while any plugin spec is in error', function()
+    use_env()
+    arti.setup({
+      auto_sync = false,
+      plugins = {
+        { 'owner/keep.nvim', tag = 'v1' },
+        { 'owner/broken.nvim' },
+      },
+    })
+    install_fake('keep.nvim', 'start')
+    install_fake('broken.nvim', 'start')
+    install_fake('orphan.nvim', 'start')
+
+    arti.clean()
+
+    t.truthy(
+      fs.exists(env.pack_dir .. '/start/broken.nvim/marker.lua'),
+      'a still-configured plugin survives its own spec error'
+    )
+    t.truthy(fs.exists(env.pack_dir .. '/start/keep.nvim'))
+    t.truthy(fs.exists(env.pack_dir .. '/start/orphan.nvim'), 'nothing is deleted from a partial spec set')
+    t.eq({ 'broken.nvim', 'keep.nvim', 'orphan.nvim' }, lock():names(), 'lock entries are left alone')
+    t.matches('missing a tag', notices(), 'the parse error is reported')
+    t.matches('clean aborted', last_notice())
+  end)
+end)
+
+t.describe('arti-pack.list', function()
+  local function line_for(lines, needle)
+    for _, line in ipairs(lines) do
+      if line:find(needle, 1, true) then
+        return line
+      end
+    end
+    return nil
+  end
+
+  t.it('says so when nothing is configured', function()
+    use_env()
+    arti.setup({ auto_sync = false })
+    t.eq({ 'no plugins configured' }, arti.list())
+  end)
+
+  t.it('reports the state of every configured and locked plugin', function()
+    use_env()
+    arti.setup({
+      auto_sync = false,
+      plugins = {
+        { 'owner/here.nvim', tag = 'v1' },
+        { 'owner/old.nvim', tag = 'v2' },
+        { 'owner/gone.nvim', tag = 'v1' },
+        { 'owner/new.nvim', tag = 'v1' },
+      },
+    })
+    install_fake('here.nvim', 'start', 'v1')
+    install_fake('old.nvim', 'start', 'v1')
+    record('gone.nvim', 'v1')
+    record('orphan.nvim', 'v1')
+
+    local lines = arti.list()
+
+    t.matches('installed$', line_for(lines, 'owner/here.nvim'))
+    t.matches('outdated %(v1%)$', line_for(lines, 'owner/old.nvim'))
+    t.matches('gone from disk$', line_for(lines, 'owner/gone.nvim'))
+    t.matches('missing$', line_for(lines, 'owner/new.nvim'))
+    t.matches('orphaned$', line_for(lines, 'orphan.nvim'))
+    t.eq(5, #lines)
+  end)
+
+  t.it('lists spec errors alongside the plugins', function()
+    use_env()
+    arti.setup({ auto_sync = false, plugins = { { 'owner/ok.nvim', tag = 'v1' }, 'bogus' } })
+
+    local lines = arti.list()
+
+    t.eq(2, #lines)
+    t.matches('^error: plugins%[2%]', line_for(lines, 'error:'))
+  end)
+end)
+
+vim.fn.stdpath = real_stdpath
+vim.notify = real_notify
diff --git a/tests/run.lua b/tests/run.lua
index a0bb7b5..ff6d6f5 100644
--- a/tests/run.lua
+++ b/tests/run.lua
@@ -8,7 +8,7 @@ package.path = table.concat({
 
 local t = require('tests.harness')
 
-for _, name in ipairs({ 'spec_spec', 'fs_spec', 'lock_spec', 'installer_spec' }) do
+for _, name in ipairs({ 'spec_spec', 'fs_spec', 'lock_spec', 'installer_spec', 'init_spec' }) do
   require('tests.' .. name)
 end