From 22be1e819a4009a6741b612cde9fc9d45a597877 Mon Sep 17 00:00:00 2001 From: dsh-slopo agent Date: Tue, 22 Sep 2026 13:23:49 +0100 Subject: [PATCH] Test the scope defaults, the backlog gate and the database reader The gate tests use a canned shell runner, and two of them build a real slopo-shaped SQLite file to prove the warm-database hole is closed: the summary line says zero new units while the database says 8,607 unembedded, and the gate must believe the database. Those fixtures insert in one transaction. Row-by-row they were 8,607 individual fsyncs, which took two minutes and timed the suite out - worth knowing before writing another one. --- test/client.test.js | 4 +- test/plugin.test.js | 187 +++++++++++++++++++++++++++++++++++++++++++- test/scope.test.js | 146 ++++++++++++++++++++++++++++++++++ 3 files changed, 334 insertions(+), 3 deletions(-) create mode 100644 test/scope.test.js diff --git a/test/client.test.js b/test/client.test.js index 811577b..e6d016c 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -191,7 +191,7 @@ test('the card renders a control for every setting the Host accepts', () => { 'Extra embedding parameters', 'slopo executable', 'Source directory', - 'Exclude patterns', + 'Extra exclude patterns', 'State directory', 'Ignore file', 'Foreground timeout', @@ -253,7 +253,7 @@ test('clearing a field is an unset, so it re-inherits rather than becoming empty test('an exclude list round-trips through comma-separated text', async () => { const { draw, react } = registered() const scope = fakeScope({ ...READY, value: { embeddingModel: 'm', embeddingDimensions: 1 } }) - inputFor(draw(scope), 'Exclude patterns').props.onChange({ target: { value: '**/test/**, *.min.js' } }) + inputFor(draw(scope), 'Extra exclude patterns').props.onChange({ target: { value: '**/test/**, *.min.js' } }) react.reset() await buttonStartingWith(draw(scope), 'Save').props.onClick() assert.deepEqual(scope.calls.set, [['sourceDirExclude', ['**/test/**', '*.min.js']]]) diff --git a/test/plugin.test.js b/test/plugin.test.js index a9cd763..ce95fc0 100644 --- a/test/plugin.test.js +++ b/test/plugin.test.js @@ -35,6 +35,8 @@ const FULL_CONFIG = { stateDir: '', embeddingRequestDelay: 0, timeoutMs: 600_000, + useDefaultExcludes: true, + maxIndexUnits: 5000, } /** A shell executor that really runs the command. */ @@ -76,7 +78,7 @@ const shell = { * `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, withShell = true, sandboxMode, sandboxPolicyResolver } = {}) { +function harness(overrides = {}, { withSettings = true, withJobs = false, withShell = true, sandboxMode, sandboxPolicyResolver, shellRun } = {}) { const config = { ...FULL_CONFIG, ...overrides } // A per-harness shell, so a test can vary sandboxMode and every resolved // request is recorded for assertion. @@ -87,6 +89,8 @@ function harness(overrides = {}, { withSettings = true, withJobs = false, withSh state.shellRequests.push(request) return { workdir: process.cwd(), timeoutMs: 600_000, stdoutMaxBytes: 10_000_000, ...request } }, + // A canned runner lets a test drive the index gate without a real slopo. + ...(shellRun === undefined ? {} : { run: shellRun }), } const state = { tools: new Map(), @@ -533,3 +537,184 @@ test('an action refused before it runs is lossless JSON', async () => { assertLossless('review (state outside sandbox)', value) assert.equal(value.errorCode, 'STATE_OUTSIDE_SANDBOX') }) + +/** A shell runner returning canned slopo output, recording every command. */ +function cannedShell(ran, { indexOutput, actionOutput = 'cluster 1:\n * /x/y.py:1-5', exitCode = 0 } = {}) { + return async (spec) => { + ran.push(spec.command) + // The command is shell-quoted, so a bare \sindex\s cannot match `'index'`. + const isIndex = /\bindex\b/.test(spec.command) + return { + exitCode: isIndex || exitCode === 0 ? 0 : exitCode, + aborted: false, + timedOut: false, + timeoutMs: spec.timeoutMs, + stdout: { text: isIndex ? indexOutput : actionOutput, truncated: false }, + stderr: { text: '', truncated: false }, + } + } +} + +test('a scope over the budget is refused BEFORE anything is embedded', async () => { + // The regression: the plugin indexed the whole repository — node_modules and + // all, 113,768 units — and began an embed that would have run for about eight + // hours. Slopo's own >1000 guard cannot catch this, because it measures the + // pre-existing backlog and a fresh database has none. + const ran = [] + const { state } = harness( + { embeddingModel: 'ollama/example:latest', embeddingDimensions: 768 }, + { shellRun: cannedShell(ran, { indexOutput: 'Indexed 113768 code units from 1300 files (0 unchanged, 0 removed).' }) }, + ) + const value = await state.tools.get('slopo').execute({ action: 'review', foreground: true }, execContext('/tmp')) + + assert.equal(value.ok, false) + assert.equal(value.errorCode, 'SCOPE_TOO_LARGE') + assert.match(value.message, /113,768 code units/) + assert.match(value.message, /still need embedding/) + assert.match(value.remedy, /sourceDirExclude/) + assert.equal(value.details.counted.units, 113_768) + assertLossless('scope refusal', value) + + // The point of the gate: the expensive step never started. + assert.equal(ran.length, 1, `only the index step may run, ran: ${ran.join(' | ')}`) + assert.match(ran[0], /\bindex\b/) + assert.doesNotMatch(ran[0], /agent-review/) +}) + +test('a scope under the budget indexes and then runs the action', async () => { + const ran = [] + const { state } = harness( + { embeddingModel: 'ollama/example:latest', embeddingDimensions: 768 }, + { shellRun: cannedShell(ran, { indexOutput: 'Indexed 1519 code units from 165 files (0 unchanged, 0 removed).' }) }, + ) + const value = await state.tools.get('slopo').execute({ action: 'review', foreground: true }, execContext('/tmp')) + assert.equal(value.ok, true) + assert.match(value.report, /cluster 1/) + assert.equal(ran.length, 2) + assert.match(ran[0], /\bindex\b/) + assert.match(ran[1], /agent-review/) +}) + +test('raising the budget lets the same scope through', async () => { + const ran = [] + const { state } = harness( + { embeddingModel: 'ollama/example:latest', embeddingDimensions: 768, maxIndexUnits: 0 }, + { shellRun: cannedShell(ran, { indexOutput: 'Indexed 113768 code units from 1300 files.' }) }, + ) + const value = await state.tools.get('slopo').execute({ action: 'review', foreground: true }, execContext('/tmp')) + assert.equal(value.ok, true) + assert.equal(ran.length, 2) +}) + +test('the config slopo is handed excludes the defaults, and a user list is added', async () => { + const ran = [] + const { state } = harness( + { + embeddingModel: 'ollama/example:latest', + embeddingDimensions: 768, + sourceDirExclude: ['ui/**', 'src/controllers/**'], + }, + { shellRun: cannedShell(ran, { indexOutput: 'Indexed 1519 code units from 165 files.' }) }, + ) + const value = await state.tools.get('slopo').execute({ action: 'review', foreground: true }, execContext('/tmp')) + const { readFileSync } = await import('node:fs') + const written = readFileSync(value.configPath, 'utf8') + for (const pattern of ['**/node_modules/**', 'ui/**', 'src/controllers/**']) { + assert.ok(written.includes(pattern), `the config slopo reads must exclude ${pattern}`) + } +}) + +test('slopo never sees a directory it should not, even when it is asked twice', async () => { + // Idempotence matters because indexing now runs on every action. + const ran = [] + const { state } = harness( + { embeddingModel: 'ollama/example:latest', embeddingDimensions: 768 }, + { shellRun: cannedShell(ran, { indexOutput: 'Indexed 1519 code units from 165 files.' }) }, + ) + const tool = state.tools.get('slopo') + await tool.execute({ action: 'prepare', foreground: true }, execContext('/tmp')) + const afterPrepare = ran.length + assert.match(ran[0], /\bindex\b/) + assert.match(ran[1], /\bembed\b/) + assert.equal(afterPrepare, 2) +}) + +test('status reports the excludes and the budget, so the levers are visible', async () => { + const { state } = harness({ + embeddingModel: 'ollama/example:latest', + embeddingDimensions: 768, + sourceDirExclude: ['ui/**'], + }) + const value = await state.tools.get('slopo').execute({ action: 'status' }, execContext('/tmp')) + assert.ok(value.details.sourceDirExclude.includes('**/node_modules/**')) + assert.ok(value.details.sourceDirExclude.includes('ui/**')) + assert.equal(value.details.useDefaultExcludes, true) + assert.equal(value.details.maxIndexUnits, 5000) + assertLossless('status', value) +}) + +test('the gate reads the DATABASE, so a warm index cannot hide a full backlog', async () => { + // The hole this closes: after a refusal the database exists with the whole + // scope unembedded, so the next run's `index` reports zero new units and a + // summary-line gate would wave the hours-long embed straight through. + const { mkdtempSync } = await import('node:fs') + const { tmpdir } = await import('node:os') + const { join } = await import('node:path') + const { DatabaseSync } = await import('node:sqlite') + + const stateDir = mkdtempSync(join(tmpdir(), 'dsh-slopo-warm-')) + const db = new DatabaseSync(join(stateDir, 'slopo.db')) + for (const table of ['files', 'code_units', 'embeddings']) db.exec(`create table ${table} (id integer primary key)`) + const fill = (table, count) => { + const insert = db.prepare(`insert into ${table} (id) values (?)`) + for (let index = 0; index < count; index += 1) insert.run(index) + } + db.exec('begin') + fill('files', 562) + fill('code_units', 8607) + db.exec('commit') + db.close() + + const ran = [] + const { state } = harness( + { embeddingModel: 'ollama/example:latest', embeddingDimensions: 768, stateDir }, + { shellRun: cannedShell(ran, { indexOutput: 'Indexed 0 code units from 0 files (562 unchanged, 0 removed).' }) }, + ) + const value = await state.tools.get('slopo').execute({ action: 'prepare', foreground: true }, execContext('/tmp')) + + assert.equal(value.errorCode, 'SCOPE_TOO_LARGE', 'the summary line said zero, the database said 8,607') + assert.equal(value.details.counted.units, 8607) + assert.equal(value.details.counted.embedded, 0) + assert.equal(ran.length, 1, `only the index step may run, ran: ${ran.join(' | ')}`) + assertLossless('warm-database refusal', value) +}) + +test('a fully embedded index has no backlog and is never refused', async () => { + const { mkdtempSync } = await import('node:fs') + const { tmpdir } = await import('node:os') + const { join } = await import('node:path') + const { DatabaseSync } = await import('node:sqlite') + + const stateDir = mkdtempSync(join(tmpdir(), 'dsh-slopo-done-')) + const db = new DatabaseSync(join(stateDir, 'slopo.db')) + for (const table of ['files', 'code_units', 'embeddings']) db.exec(`create table ${table} (id integer primary key)`) + const fill = (table, count) => { + const insert = db.prepare(`insert into ${table} (id) values (?)`) + for (let index = 0; index < count; index += 1) insert.run(index) + } + db.exec('begin') + fill('files', 5000) + fill('code_units', 50000) + fill('embeddings', 50000) + db.exec('commit') + db.close() + + const ran = [] + const { state } = harness( + { embeddingModel: 'ollama/example:latest', embeddingDimensions: 768, stateDir }, + { shellRun: cannedShell(ran, { indexOutput: 'Indexed 0 code units from 0 files (562 unchanged, 0 removed).' }) }, + ) + const value = await state.tools.get('slopo').execute({ action: 'review', foreground: true }, execContext('/tmp')) + assert.equal(value.ok, true, JSON.stringify(value)) + assert.equal(ran.length, 2, 'a done index goes straight to the action') +}) diff --git a/test/scope.test.js b/test/scope.test.js new file mode 100644 index 0000000..23c916a --- /dev/null +++ b/test/scope.test.js @@ -0,0 +1,146 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' + +import { + DEFAULT_SOURCE_EXCLUDES, + budgetRemedy, + checkBudget, + effectiveExcludes, + estimateDuration, + parseIndexOutput, + readIndexCounts, +} from '../lib/scope.js' + +test('default excludes cover the vendored and generated directories', () => { + // The measured 13x blow-up came from node_modules alone; these are the + // defaults that must exist for the gate to be reachable at all. + for (const pattern of ['**/node_modules/**', '**/.git/**', '**/dist/**', '**/__pycache__/**']) { + assert.ok(DEFAULT_SOURCE_EXCLUDES.includes(pattern), `missing default exclude ${pattern}`) + } +}) + +test('defaults apply unless explicitly turned off', () => { + assert.deepEqual(effectiveExcludes({}), [...DEFAULT_SOURCE_EXCLUDES]) + assert.deepEqual(effectiveExcludes({ useDefaultExcludes: true }), [...DEFAULT_SOURCE_EXCLUDES]) + assert.deepEqual(effectiveExcludes({ useDefaultExcludes: false }), []) +}) + +test('a user list is ADDED to the defaults, not substituted for them', () => { + // A settings patch replaces a whole config value, so if sourceDirExclude were + // the only knob, setting it to ['ui/**'] would silently stop excluding + // node_modules and bring the 114k-unit embed straight back. + const excludes = effectiveExcludes({ sourceDirExclude: ['ui/**', 'src/controllers/**'] }) + assert.ok(excludes.includes('**/node_modules/**')) + assert.ok(excludes.includes('ui/**')) + assert.ok(excludes.includes('src/controllers/**')) +}) + +test('duplicates collapse and junk entries are dropped', () => { + const excludes = effectiveExcludes({ + sourceDirExclude: ['ui/**', 'ui/**', '', 42, null, '**/node_modules/**'], + }) + assert.equal(excludes.filter((entry) => entry === 'ui/**').length, 1) + assert.equal(excludes.filter((entry) => entry === '**/node_modules/**').length, 1) + assert.ok(excludes.every((entry) => typeof entry === 'string' && entry !== '')) +}) + +test('parseIndexOutput reads slopo 0.8.0s own summary line', () => { + assert.deepEqual(parseIndexOutput('Indexed 3 code units from 2 files (0 unchanged, 0 removed).'), { + units: 3, + files: 2, + }) + assert.deepEqual(parseIndexOutput('indexing...\nIndexed 113,768 code units from 2,104 files'), { + units: 113768, + files: 2104, + }) +}) + +test('parseIndexOutput returns undefined rather than guessing', () => { + assert.equal(parseIndexOutput(''), undefined) + assert.equal(parseIndexOutput('Indexing code...\n'), undefined) + assert.equal(parseIndexOutput(undefined), undefined) +}) + +test('checkBudget allows anything at or under the budget', () => { + assert.equal(checkBudget({ units: 1519, files: 165 }, 5000, []).allowed, true) + assert.equal(checkBudget({ units: 5000, files: 100 }, 5000, []).allowed, true) +}) + +test('checkBudget refuses an oversized scope and says the numbers', () => { + const decision = checkBudget({ units: 113_768, files: 1300 }, 5000, []) + assert.equal(decision.allowed, false) + assert.match(decision.reason, /113,768 code units/) + assert.match(decision.reason, /1,300 files/) + assert.match(decision.reason, /hours/) + assert.match(decision.reason, /against a budget of 5,000/) +}) + +test('a budget of zero removes the limit, and an unreadable count is never a refusal', () => { + assert.equal(checkBudget({ units: 999_999, files: 1 }, 0, []).allowed, true) + assert.equal(checkBudget(undefined, 5000, []).allowed, true) + assert.equal(checkBudget({ units: 999_999, files: 1 }, Number.NaN, []).allowed, true) +}) + +test('estimateDuration is human-scaled', () => { + assert.match(estimateDuration(40), /seconds/) + assert.match(estimateDuration(1519), /minutes/) + assert.match(estimateDuration(113_768), /hours/) +}) + +test('the remedy names both levers and the current state', () => { + const many = budgetRemedy(5000, ['a', 'b']) + assert.match(many, /sourceDirExclude/) + assert.match(many, /2 patterns are excluded/) + assert.match(many, /maxIndexUnits/) + assert.match(many, /node_modules/) + assert.match(budgetRemedy(5000, []), /nothing is currently excluded/) +}) + +/** Build a throwaway slopo-shaped database. */ +async function fakeDb({ units, embedded, files }) { + const { mkdtempSync } = await import('node:fs') + const { tmpdir } = await import('node:os') + const { join } = await import('node:path') + const { DatabaseSync } = await import('node:sqlite') + const dir = mkdtempSync(join(tmpdir(), 'dsh-slopo-db-')) + const path = join(dir, 'slopo.db') + const db = new DatabaseSync(path) + for (const table of ['files', 'code_units', 'embeddings']) db.exec(`create table ${table} (id integer primary key)`) + const fill = (table, count) => { + const insert = db.prepare(`insert into ${table} (id) values (?)`) + for (let index = 0; index < count; index += 1) insert.run(index) + } + // One transaction: without it each insert is its own fsync, and 8,607 of + // them on this filesystem took two minutes. + db.exec('begin') + fill('files', files) + fill('code_units', units) + fill('embeddings', embedded) + db.exec('commit') + db.close() + return path +} + +test('readIndexCounts reads the real backlog out of the database', async () => { + // This is the signal the gate actually needs: `Indexed N` counts only what was + // newly indexed, so it is useless on a warm database. + const path = await fakeDb({ units: 8607, embedded: 0, files: 562 }) + assert.deepEqual(readIndexCounts(path), { units: 8607, embedded: 0, unembedded: 8607, files: 562 }) +}) + +test('readIndexCounts reports what is LEFT to embed, not what is in scope', async () => { + const path = await fakeDb({ units: 1519, embedded: 1519, files: 165 }) + const counts = readIndexCounts(path) + assert.equal(counts.units, 1519) + assert.equal(counts.unembedded, 0, 'a fully embedded index has no backlog') +}) + +test('readIndexCounts returns undefined rather than throwing on a bad file', async () => { + assert.equal(readIndexCounts('/nonexistent/definitely/not/here.db'), undefined) + const { mkdtempSync, writeFileSync } = await import('node:fs') + const { tmpdir } = await import('node:os') + const { join } = await import('node:path') + const notADb = join(mkdtempSync(join(tmpdir(), 'dsh-slopo-junk-')), 'junk.db') + writeFileSync(notADb, 'this is not a sqlite file') + assert.equal(readIndexCounts(notADb), undefined) +})