Test the plugin against the real harness seams and the real CLI

The integration tests load the real plugin module against a thin mock Cordis
context and drive the tool through a real shell into the real slopo binary. The
mock implements only the seams the plugin touches, so a wrong assumption about a
dsh service fails a test instead of producing a plugin that silently does
nothing at runtime.

Two real bugs were caught this way and are now pinned:

  * renderSlopoConfig would write the embedding API key to disk if it were
    handed one. It cannot now.
  * --config-version was placed before the subcommand, which slopo rejects with
    exit code 2. It is declared on the command, not the callback.

The end-to-end test is opt-in (DSH_SLOPO_E2E=1) because it needs a reachable
embedding backend; it indexes a real repository, embeds it, and asserts a real
duplicate cluster comes back.

test/plugin.test.js

@@ -0,0 +1,325 @@
+/**
+ * Integration tests: load the real plugin module against a mock Cordis context,
+ * then drive the tool through a real shell into the real slopo CLI.
+ *
+ * The mock is deliberately thin — it implements only the seams the plugin
+ * actually touches — so a wrong assumption about a dsh service shows up here as
+ * a failing test rather than as a plugin that silently does nothing at runtime.
+ */
+
+import assert from 'node:assert/strict'
+import { exec } from 'node:child_process'
+import { existsSync, mkdtempSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { test } from 'node:test'
+
+import { findSlopo } from '../lib/slopo.js'
+
+const testHome = mkdtempSync(join(tmpdir(), 'dsh-slopo-home-'))
+process.env.DSH_HOME = testHome
+
+const { Config, apply } = await import('../index.js')
+
+const FULL_CONFIG = {
+  binary: '',
+  autoInstall: true,
+  embeddingModel: '',
+  embeddingDimensions: 0,
+  embeddingApiBase: '',
+  embeddingApiKey: '',
+  embeddingParamsJson: '{}',
+  sourceDir: '',
+  sourceDirExclude: [],
+  ignoreFile: '',
+  stateDir: '',
+  embeddingRequestDelay: 0,
+  timeoutMs: 600_000,
+}
+
+/** A shell executor that really runs the command. */
+const shell = {
+  sandboxMode: undefined,
+  resolve: (request) => ({
+    workdir: process.cwd(),
+    timeoutMs: 600_000,
+    stdoutMaxBytes: 10_000_000,
+    ...request,
+  }),
+  run(spec) {
+    return new Promise((resolve) => {
+      exec(
+        spec.command,
+        { cwd: spec.workdir, env: { ...process.env, ...(spec.env ?? {}) }, timeout: spec.timeoutMs, maxBuffer: 64 * 1024 * 1024 },
+        (error, stdout, stderr) => {
+          resolve({
+            exitCode: error === null ? 0 : typeof error.code === 'number' ? error.code : 1,
+            signal: null,
+            timedOut: Boolean(error?.killed),
+            aborted: false,
+            timeoutMs: spec.timeoutMs,
+            stdout: { text: String(stdout), truncated: false },
+            stderr: { text: String(stderr), truncated: false },
+          })
+        },
+      )
+    })
+  },
+  start() {
+    throw new Error('background process start is not used by these tests')
+  },
+}
+
+/**
+ * Build a context exposing exactly the services the plugin consumes.
+ *
+ * `jobs` is deliberately absent, which forces the tool down its foreground path
+ * so the assertions can see a result rather than a job handle.
+ */
+function harness(overrides = {}, { withSettings = true, withJobs = false } = {}) {
+  const config = { ...FULL_CONFIG, ...overrides }
+  const state = { tools: new Map(), updates: [], provider: undefined, namespace: undefined, shell, jobs: undefined }
+
+  const ctx = {
+    logger: () => ({ info() {}, warn() {}, debug() {}, error() {} }),
+    get(name) {
+      if (name === 'shell') return shell
+      if (name === 'jobs') return withJobs ? state.jobs : undefined
+      return undefined
+    },
+    inject(deps, callback) {
+      if (!withSettings || !deps.includes('settings')) return
+      callback({
+        settings: {
+          installSection(owner, namespace, schema, entry, hooks) {
+            state.namespace = namespace
+            state.schema = schema
+            hooks.setSource(() => config)
+          },
+          async update(namespace, patch) {
+            state.updates.push({ namespace, patch })
+            Object.assign(config, patch)
+          },
+        },
+      })
+    },
+    skills: {
+      registerProvider(create) {
+        state.provider = create({ signal: new AbortController().signal, invalidate() {} })
+      },
+    },
+    tools: {
+      register(definition) {
+        state.tools.set(definition.name, definition)
+      },
+    },
+  }
+
+  apply(ctx, config)
+  return { config, state }
+}
+
+const execContext = (cwd) => ({ signal: new AbortController().signal, agent: { session: { header: { cwd } } } })
+
+test('Config is a schemastery schema, which is what cordis requires', () => {
+  assert.equal(typeof Config, 'function')
+  assert.ok(Config['~standard'] !== undefined, 'expected a Standard Schema')
+})
+
+test('apply registers exactly one tool, and it is called slopo', () => {
+  const { state } = harness()
+  assert.deepEqual([...state.tools.keys()], ['slopo'])
+})
+
+test('the ported skills are published user-invocable and NOT model-invocable', async () => {
+  const { state } = harness()
+  const candidates = await state.provider.list()
+  assert.deepEqual(
+    candidates.map((c) => c.name),
+    ['slopo-review', 'slopo-analyze-ignore', 'slopo-analyze-one'],
+  )
+  for (const candidate of candidates) {
+    assert.equal(candidate.invocation.userInvocable, true, candidate.name)
+    assert.equal(candidate.invocation.modelInvocable, false, candidate.name)
+    assert.equal(candidate.rank, 600)
+    assert.equal(candidate.source, 'bundled')
+    assert.ok(candidate.description.length > 20, candidate.name)
+    assert.ok(candidate.locator !== undefined)
+  }
+})
+
+test('get() returns the skill body, routed through the locator list() issued', async () => {
+  const { state } = harness()
+  const [candidate] = await state.provider.list()
+  const skill = await state.provider.get(candidate)
+  assert.equal(skill.name, 'slopo-review')
+  assert.match(skill.content, /# Instructions/)
+  assert.match(skill.content, /First-turn output rules/)
+  assert.match(skill.content, /slopo.*action: "review"/s)
+})
+
+test('every skill tells the agent to call the tool rather than a shell command', async () => {
+  const { state } = harness()
+  for (const candidate of await state.provider.list()) {
+    const skill = await state.provider.get(candidate)
+    assert.match(skill.content, /`slopo` tool/, candidate.name)
+    // The Claude Code bash-injection directive must not survive the port.
+    assert.doesNotMatch(skill.content, /!`/, candidate.name)
+    assert.doesNotMatch(skill.content, /allowed-tools/, candidate.name)
+  }
+})
+
+test('the settings namespace is registered under a grammar-valid name', () => {
+  const { state } = harness()
+  assert.match(state.namespace, /^[a-z][a-z0-9-]*$/)
+})
+
+test('status reports the missing settings instead of pretending to be ready', async () => {
+  const { state } = harness()
+  const tool = state.tools.get('slopo')
+  const value = await tool.execute({ action: 'status' }, execContext('/tmp'))
+  assert.equal(value.kind, 'foreground')
+  assert.equal(value.ok, false)
+  assert.deepEqual(value.details.missingSettings, ['embedding_model', 'embedding_dimensions'])
+  assert.match(value.message, /Not configured yet/)
+})
+
+test('status resolves state under $DSH_HOME, never inside the workspace', async () => {
+  const { state } = harness()
+  const tool = state.tools.get('slopo')
+  const value = await tool.execute({ action: 'status' }, execContext('/tmp/some-repo'))
+  assert.ok(value.details.stateDir.startsWith(testHome), value.details.stateDir)
+  assert.doesNotMatch(value.details.stateDir, /^\/tmp\/some-repo/)
+})
+
+test('configure rejects a setting this plugin does not have', async () => {
+  const { state } = harness()
+  const tool = state.tools.get('slopo')
+  const value = await tool.execute({ action: 'configure', config: { embeddingModel: 'x', nope: 1 } }, execContext('/tmp'))
+  assert.equal(value.ok, false)
+  assert.equal(value.errorCode, 'UNKNOWN_SETTING')
+  assert.match(value.message, /nope/)
+  assert.deepEqual(state.updates, [])
+})
+
+test('configure persists through the settings namespace, not a private file', async () => {
+  const { state } = harness()
+  const tool = state.tools.get('slopo')
+  const value = await tool.execute(
+    { action: 'configure', config: { embeddingModel: 'ollama/m:latest', embeddingDimensions: 768 } },
+    execContext('/tmp'),
+  )
+  assert.equal(value.ok, true)
+  assert.equal(state.updates.length, 1)
+  assert.equal(state.updates[0].namespace, 'dsh-slopo')
+  assert.deepEqual(state.updates[0].patch, { embeddingModel: 'ollama/m:latest', embeddingDimensions: 768 })
+})
+
+test('configure says so plainly when no settings provider is mounted', async () => {
+  const { state } = harness({}, { withSettings: false })
+  const tool = state.tools.get('slopo')
+  const value = await tool.execute({ action: 'configure', config: { embeddingModel: 'x' } }, execContext('/tmp'))
+  assert.equal(value.ok, false)
+  assert.equal(value.errorCode, 'NO_SETTINGS_PROVIDER')
+})
+
+test('the plugin still loads and registers its tool without a settings provider', () => {
+  // This is the whole reason `settings` goes through ctx.inject rather than the
+  // top-level inject: a required dependency would have suppressed apply entirely.
+  const { state } = harness({}, { withSettings: false })
+  assert.ok(state.tools.has('slopo'))
+  assert.ok(state.provider !== undefined)
+})
+
+test('review refuses to run before the workspace is configured', async () => {
+  const { state } = harness()
+  const tool = state.tools.get('slopo')
+  const value = await tool.execute({ action: 'review' }, execContext('/tmp'))
+  assert.equal(value.ok, false)
+  assert.equal(value.errorCode, 'NOT_CONFIGURED')
+  assert.match(value.remedy, /configure/)
+})
+
+test('review writes a config the plugin owns and passes the key only in the environment', async () => {
+  const { state } = harness({
+    embeddingModel: 'ollama/example:latest',
+    embeddingDimensions: 768,
+    embeddingApiBase: 'http://127.0.0.1:11434',
+    embeddingApiKey: 'super-secret',
+  })
+  const tool = state.tools.get('slopo')
+  const value = await tool.execute({ action: 'review', foreground: true }, execContext('/tmp/nonexistent-repo-xyz'))
+  // It will fail — there is no git repo there — but the failure must be a
+  // reported one, and the config must have been materialised first.
+  assert.equal(value.kind, 'foreground')
+  assert.ok(existsSync(value.configPath), `expected config at ${value.configPath}`)
+  const { readFileSync } = await import('node:fs')
+  const written = readFileSync(value.configPath, 'utf8')
+  assert.match(written, /embedding_model: "ollama\/example:latest"/)
+  assert.doesNotMatch(written, /super-secret/)
+})
+
+test('a background-capable profile gets a job handle instead of a blocking call', async () => {
+  const started = []
+  const { state } = harness(
+    { embeddingModel: 'ollama/example:latest', embeddingDimensions: 768 },
+    { withJobs: true },
+  )
+  // ctx.get('jobs') is consulted at call time, not at apply time, so the fake
+  // registry can be installed after the plugin has loaded.
+  state.jobs = {
+    start(spec) {
+      started.push(spec)
+      return 'slopo-1'
+    },
+  }
+  const tool = state.tools.get('slopo')
+  const value = await tool.execute({ action: 'review' }, execContext('/tmp'))
+  assert.equal(value.kind, 'background')
+  assert.equal(value.jobId, 'slopo-1')
+  assert.equal(started.length, 1)
+  assert.equal(started[0].kind, 'slopo')
+  assert.equal(typeof started[0].run, 'function')
+})
+
+test(
+  'review runs the real slopo CLI end to end against a real repository',
+  { skip: process.env.DSH_SLOPO_E2E !== '1' || findSlopo(testHome) === undefined },
+  async () => {
+    const repo = mkdtempSync(join(tmpdir(), 'dsh-slopo-e2e-'))
+    const { execFileSync } = await import('node:child_process')
+    const { mkdirSync } = await import('node:fs')
+    mkdirSync(join(repo, 'src'), { recursive: true })
+    writeFileSync(
+      join(repo, 'src', 'orders.py'),
+      'def total_for_order(order):\n    total = 0\n    for line in order["lines"]:\n        total += line["price"] * line["qty"]\n    return round(total, 2)\n',
+    )
+    writeFileSync(
+      join(repo, 'src', 'invoices.py'),
+      'def invoice_total(invoice):\n    running = 0\n    for item in invoice["items"]:\n        running += item["unit_price"] * item["quantity"]\n    return round(running, 2)\n',
+    )
+    execFileSync('git', ['init', '-q'], { cwd: repo })
+    execFileSync('git', ['add', '-A'], { cwd: repo })
+    execFileSync('git', ['-c', 'user.email=t@t', '-c', 'user.name=t', 'commit', '-qm', 'init'], { cwd: repo })
+    writeFileSync(
+      join(repo, 'src', 'orders.py'),
+      'def total_for_order(order):\n    total = 0\n    for line in order["lines"]:\n        total += line["price"] * line["qty"]\n    return round(total, 2)\n\n\ndef refund_for_order(order):\n    amount = 0\n    for line in order["lines"]:\n        amount += line["price"] * line["qty"]\n    return round(amount, 2)\n',
+    )
+
+    const { state } = harness({
+      sourceDir: join(repo, 'src'),
+      embeddingModel: 'ollama/unclemusclez/jina-embeddings-v2-base-code:latest',
+      embeddingDimensions: 768,
+      embeddingApiBase: 'http://10.50.1.2:11434',
+    })
+    const tool = state.tools.get('slopo')
+
+    const prepared = await tool.execute({ action: 'prepare', foreground: true }, execContext(repo))
+    assert.equal(prepared.ok, true, JSON.stringify(prepared))
+
+    const reviewed = await tool.execute({ action: 'review', foreground: true }, execContext(repo))
+    assert.equal(reviewed.ok, true, JSON.stringify(reviewed))
+    assert.match(reviewed.report, /cluster 1/)
+    assert.match(reviewed.report, /\* /)
+  },
+)