From ef903a0c18796dcb7f261d52a8ef0867b4dff4fd Mon Sep 17 00:00:00 2001 From: Dodothereal <129273127+Dodothereal@users.noreply.github.com> Date: Mon, 26 Oct 2026 10:12:35 +0100 Subject: [PATCH 1/3] fix(dx): sort the registry and derive the artifact tool count The registry declared an alphabetical-by-tool-name invariant and broke it: mandates_and_justice sorts before map_overview, so the entries (and their imports) were swapped back into order. smoke-artifact.mjs also pinned the expected tool count to the literal 45, so the next tool to land would fail the release gate until someone remembered the constant. The count now comes from the registry itself: the build emits dist/surface.js from ALL_TOOLS through the same isGatedOff predicate the server uses, and the smoke imports it. That keeps the release check working on Node 20, which cannot load the .ts registry (verify.mjs stays Node-24-only). Refs #70. --- CHANGELOG.md | 7 +++++++ scripts/smoke-artifact.mjs | 6 +++++- src/surface.ts | 6 ++++++ src/tools/registry.ts | 4 ++-- test/toolsurface.test.mjs | 39 ++++++++++++++++++++++++++++++++++++++ tsup.config.ts | 2 +- 6 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 src/surface.ts create mode 100644 test/toolsurface.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f73314..22a9bc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,13 @@ backwards-compatible fixes only. ## [Unreleased] +### Fixed + +- The registry is alphabetical again (`mandates_and_justice` sorts before + `map_overview`), and a unit test now fails if that order drifts. +- The release smoke check derives its expected tool count from `ALL_TOOLS` + instead of pinning a literal, so adding a tool no longer breaks it. + ## [1.4.1] - 2026-07-26 ### Fixed diff --git a/scripts/smoke-artifact.mjs b/scripts/smoke-artifact.mjs index 6d917a3..6645c15 100644 --- a/scripts/smoke-artifact.mjs +++ b/scripts/smoke-artifact.mjs @@ -20,6 +20,9 @@ try { writeFileSync(join(room, 'package.json'), JSON.stringify({ name: 'smoke', private: true })); run(`npm install "${tgz}"`, room); const bin = join(room, 'node_modules', 'dfhack-mcp', 'dist', 'index.js'); + const { SURFACE_ACTUATORS_ON } = await import( + join(room, 'node_modules', 'dfhack-mcp', 'dist', 'surface.js') + ); const childEnv = { ...process.env, DFHACK_MCP_ACTUATORS: '1' }; delete childEnv.DFHACK_MCP_DEV; @@ -47,7 +50,8 @@ try { const problems = []; if (handshakeVersion !== version) problems.push(`handshake version ${handshakeVersion} != package ${version}`); - if (tools.length !== 45) problems.push(`expected 45 tools (actuators on), got ${tools.length}`); + if (tools.length !== SURFACE_ACTUATORS_ON) + problems.push(`expected ${SURFACE_ACTUATORS_ON} tools (actuators on), got ${tools.length}`); if (missing.length) problems.push(`missing tools: ${missing.join(', ')}`); if (problems.length) { diff --git a/src/surface.ts b/src/surface.ts new file mode 100644 index 0000000..b7e9b11 --- /dev/null +++ b/src/surface.ts @@ -0,0 +1,6 @@ +import { ALL_TOOLS } from './tools/registry.ts'; +import { isGatedOff } from './register.ts'; + +export const SURFACE_ACTUATORS_ON = ALL_TOOLS.filter((def) => + !isGatedOff(def, { DFHACK_MCP_ACTUATORS: '1' }) +).length; diff --git a/src/tools/registry.ts b/src/tools/registry.ts index 28b9947..653f3ca 100644 --- a/src/tools/registry.ts +++ b/src/tools/registry.ts @@ -19,8 +19,8 @@ import { identifyDef } from './identify/index.ts'; import { injuriesAndHealthDef } from './injuriesAndHealth.ts'; import { jobsAndLaborDef } from './jobsAndLabor.ts'; import { livestockAndPasturesDef } from './livestockAndPastures.ts'; -import { mapOverviewDef } from './mapOverview.ts'; import { mandatesAndJusticeDef } from './mandatesAndJustice.ts'; +import { mapOverviewDef } from './mapOverview.ts'; import { mechanismsDef, pullLeverDef } from './mechanisms.ts'; import { militaryDef } from './military.ts'; import { moodsDef } from './moods.ts'; @@ -64,8 +64,8 @@ export const ALL_TOOLS: ToolDef[] = [ injuriesAndHealthDef, jobsAndLaborDef, livestockAndPasturesDef, - mapOverviewDef, mandatesAndJusticeDef, + mapOverviewDef, mechanismsDef, militaryDef, moodsDef, diff --git a/test/toolsurface.test.mjs b/test/toolsurface.test.mjs new file mode 100644 index 0000000..562d55f --- /dev/null +++ b/test/toolsurface.test.mjs @@ -0,0 +1,39 @@ +// Unit tests for the tool-surface bookkeeping contract (issue #70). The registry +// must stay alphabetical by tool name so sibling tool PRs merge without churn, +// and the published artifact's expected tool count must come from that same +// registry rather than a literal that goes stale when the next tool lands. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { ALL_TOOLS } from '../src/tools/registry.ts'; + +const names = ALL_TOOLS.map((tool) => tool.name); +const root = new globalThis.URL('..', import.meta.url); +const read = (name) => readFileSync(new globalThis.URL(name, root), 'utf8'); + +test('registry entries are alphabetical by tool name', () => { + const inversions = []; + for (let i = 1; i < names.length; i += 1) { + if (names[i - 1] > names[i]) inversions.push(`${names[i - 1]} > ${names[i]}`); + } + assert.deepEqual(inversions, [], `registry out of alphabetical order: ${inversions.join('; ')}`); +}); + +test('registry tool names are unique', () => { + const duplicates = names.filter((name, index) => names.indexOf(name) !== index); + assert.deepEqual(duplicates, [], `duplicate tool names: ${duplicates.join(', ')}`); +}); + +test('smoke-artifact compares against the derived count, not a literal', () => { + const source = read('scripts/smoke-artifact.mjs'); + assert.doesNotMatch( + source, + /tools\.length\s*!==\s*\d+/, + 'smoke-artifact must not pin the expected tool count to a literal' + ); + assert.match( + source, + /dist['"],\s*['"]surface\.js|surface\.js/, + 'smoke-artifact must read the count the build derives from the registry' + ); +}); diff --git a/tsup.config.ts b/tsup.config.ts index 199da6e..1ce635b 100644 --- a/tsup.config.ts +++ b/tsup.config.ts @@ -2,7 +2,7 @@ import { defineConfig } from 'tsup'; import { cp } from 'node:fs/promises'; export default defineConfig({ - entry: ['src/index.ts'], + entry: ['src/index.ts', 'src/surface.ts'], outDir: 'dist', format: ['esm'], target: 'node20', From 734ab501d212bdc8d9386cbbcc804bde8e640cd0 Mon Sep 17 00:00:00 2001 From: Dodothereal <129273127+Dodothereal@users.noreply.github.com> Date: Mon, 26 Oct 2026 10:40:41 +0100 Subject: [PATCH 2/3] test(dx): keep only the registry property tests Drop the source-text regex test: it asserted on implementation text rather than observable behaviour, and the artifact-mutation proof already covers the derived count end to end. The file now has no comment block and no unused helpers, which matches the repo's zero-comment-block target. --- test/toolsurface.test.mjs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/test/toolsurface.test.mjs b/test/toolsurface.test.mjs index 562d55f..fdb01c1 100644 --- a/test/toolsurface.test.mjs +++ b/test/toolsurface.test.mjs @@ -1,15 +1,8 @@ -// Unit tests for the tool-surface bookkeeping contract (issue #70). The registry -// must stay alphabetical by tool name so sibling tool PRs merge without churn, -// and the published artifact's expected tool count must come from that same -// registry rather than a literal that goes stale when the next tool lands. import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { readFileSync } from 'node:fs'; import { ALL_TOOLS } from '../src/tools/registry.ts'; const names = ALL_TOOLS.map((tool) => tool.name); -const root = new globalThis.URL('..', import.meta.url); -const read = (name) => readFileSync(new globalThis.URL(name, root), 'utf8'); test('registry entries are alphabetical by tool name', () => { const inversions = []; @@ -23,17 +16,3 @@ test('registry tool names are unique', () => { const duplicates = names.filter((name, index) => names.indexOf(name) !== index); assert.deepEqual(duplicates, [], `duplicate tool names: ${duplicates.join(', ')}`); }); - -test('smoke-artifact compares against the derived count, not a literal', () => { - const source = read('scripts/smoke-artifact.mjs'); - assert.doesNotMatch( - source, - /tools\.length\s*!==\s*\d+/, - 'smoke-artifact must not pin the expected tool count to a literal' - ); - assert.match( - source, - /dist['"],\s*['"]surface\.js|surface\.js/, - 'smoke-artifact must read the count the build derives from the registry' - ); -}); From cd1402611c52f0dc3996f0a1e87e73e6291637af Mon Sep 17 00:00:00 2001 From: Dodothereal <129273127+Dodothereal@users.noreply.github.com> Date: Mon, 26 Oct 2026 10:47:15 +0100 Subject: [PATCH 3/3] fix(dx): import the derived surface through a file URL A bare filesystem path handed to import() is not a URL: on Windows a C:\ path is read as an unsupported scheme, and on POSIX a '#' or '%' in any path segment is taken as a fragment or escape. This repo is Windows-native, and TMPDIR can legitimately contain those characters, so convert with pathToFileURL first. --- scripts/smoke-artifact.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/smoke-artifact.mjs b/scripts/smoke-artifact.mjs index 6645c15..b6dc397 100644 --- a/scripts/smoke-artifact.mjs +++ b/scripts/smoke-artifact.mjs @@ -4,6 +4,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { createRequire } from 'node:module'; import { join } from 'node:path'; +import { pathToFileURL } from 'node:url'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; @@ -21,7 +22,7 @@ try { run(`npm install "${tgz}"`, room); const bin = join(room, 'node_modules', 'dfhack-mcp', 'dist', 'index.js'); const { SURFACE_ACTUATORS_ON } = await import( - join(room, 'node_modules', 'dfhack-mcp', 'dist', 'surface.js') + pathToFileURL(join(room, 'node_modules', 'dfhack-mcp', 'dist', 'surface.js')).href ); const childEnv = { ...process.env, DFHACK_MCP_ACTUATORS: '1' };