From d19146e1e2cf7a6330dd00dd474e47f4c8d3d3fb Mon Sep 17 00:00:00 2001 From: Engineer Date: Mon, 26 Oct 2026 11:41:58 -0400 Subject: [PATCH 1/7] fix: check database availability before reporting readiness Add dependency-free offline startup contracts and document failure boundaries. Co-Authored-By: Paperclip --- README.md | 6 +- docs/startup-readiness.md | 59 +++++++++++++++ package.json | 1 + scripts/startup-contract.test.cjs | 121 ++++++++++++++++++++++++++++++ server.js | 8 +- 5 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 docs/startup-readiness.md create mode 100644 scripts/startup-contract.test.cjs diff --git a/README.md b/README.md index f47b14d..061bdf6 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ See [CHANGELOG.md](CHANGELOG.md) for detailed release notes. Run `npm test` (or `npm run test:unit`) to execute Jest API/integration tests. Run `npm run test:e2e` for Playwright UI tests. Run `npm run smoke:mcp` for deterministic MCP plan/thought lifecycle smoke validation. +Run `npm run test:startup-contract` for dependency-free, injected offline startup/readiness tests. ### Production Security `/tpc.db` download is disabled in production by default. @@ -22,7 +23,10 @@ To explicitly allow it, set `EXPOSE_TPC_DB=true`. ### Operational Endpoints - `GET /health` returns basic liveness info. -- `GET /ready` returns readiness info and DB path. +- `GET /ready` returns readiness info and DB path after a successful database query, or 503 when unavailable. + +See the [startup/readiness contract](docs/startup-readiness.md) for initialization, +failure behavior, test boundaries, and limitations. ### Data Contract (REST + MCP) - **IDs**: integer autoincrement IDs from SQLite for plans/thoughts. diff --git a/docs/startup-readiness.md b/docs/startup-readiness.md new file mode 100644 index 0000000..418c166 --- /dev/null +++ b/docs/startup-readiness.md @@ -0,0 +1,59 @@ +# Startup and readiness contract + +Run the dependency-free offline contract suite with Node's built-in test runner: + +```sh +node --test scripts/startup-contract.test.cjs +# equivalent: npm run test:startup-contract +``` + +These tests execute the actual `server.js` source in a fresh VM context, injecting +Express, routers and the database boundary. They do not open sockets, load SQLite, +read application data, run MCP, or use external services. They are not an Express, +SQLite, HTTP integration, or Windows qualification test. The normal Jest suite +remains separate (`npm test`) and requires installed dependencies. + +## Initialization + +- Importing `server.js` constructs and exports the global Express app but does not + initialize its database or call `listen`. +- Running it as the main module awaits `initGlobalDB()` before `listen(PORT)`. + The promise represents backup preparation, opening SQLite and completion of + `performMigration`, not merely opening the file. Schema creation, alterations, + indexes, FTS triggers/rebuild, and baseline migration bookkeeping are awaited. +- Historical JSON imports are **best effort**: their errors are caught and logged + inside `performMigration`. Initialization resolving does not certify all legacy + JSON was imported. `skipMigration=true` skips JSON import, not schema setup. +- `getDB()` throws until global initialization has installed the handle. +- Initialization rejection is logged and prevents the main module from listening. + This is **not** a clean shutdown guarantee: the current entry point does not set + a nonzero exit status, and migration failure after opening SQLite does not close + that handle. Those boundaries are not changed by the readiness repair. + +## Health versus readiness + +- `/health` is liveness only and returns 200 without a database check. +- `/ready` returns 200 with `{ ready: true, db: }` only after `getDB()` + succeeds and SQLite completes `SELECT 1` successfully. Missing initialization, + a closed database, or another query error returns 503 with `ready: false` and + the existing error-message field. Each request checks again; readiness is not + latched after first success. +- This is point-in-time **connection/query readiness**, not proof of schema + integrity, writeability, imported records, or future query success. There is no + timeout: a query whose callback never arrives leaves the request pending. +- The isolated `createApp()` factory waits for its own database before returning + and injects it on requests, but does not mount `/health` or `/ready`. These + endpoints describe the exported global app only. + +## Regression and scope + +Previously `/ready` only joined a path and returned success, even before global +initialization or after a database query failure. The offline regression suite +reproduces that behavior on the original source and checks the repaired result, +including waiting for the query callback. Startup ordering and rejection tests +also preserve the existing no-listen-before-initialization behavior. + +No bind address, port default, database permissions, schema, authentication, +startup failure handling or production configuration was changed. Do not run the +application against real data to exercise this suite. Revert the isolated commit +to roll back; no data migration or runtime cleanup is needed for these tests. diff --git a/package.json b/package.json index 294ce91..45fdbe8 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,7 @@ "scripts": { "test": "npm run test:unit", "test:unit": "cross-env NODE_ENV=test jest --runInBand", + "test:startup-contract": "node --test scripts/startup-contract.test.cjs", "test:e2e": "playwright test", "mcp": "node mcp-server.js", "smoke:mcp": "node scripts/mcp-smoke.js", diff --git a/scripts/startup-contract.test.cjs b/scripts/startup-contract.test.cjs new file mode 100644 index 0000000..15b9c2d --- /dev/null +++ b/scripts/startup-contract.test.cjs @@ -0,0 +1,121 @@ +// Dependency-free contract tests: execute the real server module with fake boundaries. +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); + +function load({ main = false } = {}) { + const handlers = new Map(); + const calls = []; + const errors = []; + let resolveInit, rejectInit; + let database; + const initialization = new Promise((resolve, reject) => { + resolveInit = resolve; + rejectInit = reject; + }); + const app = { + use() {}, + get(route, handler) { handlers.set(route, handler); }, + listen(port, callback) { + calls.push(['listen', port]); + callback(); + return { ref() { calls.push(['ref']); } }; + } + }; + const express = () => app; + express.json = express.static = () => () => {}; + const db = { + initGlobalDB() { calls.push(['init']); return initialization; }, + cleanDB() {}, + getDB() { + if (!database) throw new Error('Global DB not initialized'); + return database; + } + }; + const module = { exports: {} }; + const requireFake = (name) => { + if (name === 'express') return express; + if (name === 'path') return path; + if (name === './db/database.js') return db; + if (name.startsWith('./routes/')) return () => {}; + if (name === './middleware/errorHandler') return { errorHandler() {} }; + throw new Error(`Unexpected import: ${name}`); + }; + requireFake.main = main ? module : {}; + const filename = path.join(__dirname, '..', 'server.js'); + vm.runInNewContext(fs.readFileSync(filename, 'utf8'), { + require: requireFake, module, __dirname: path.dirname(filename), + process: { env: { NODE_ENV: 'test' } }, + console: { log() {}, error(error) { errors.push(error); } } + }, { filename }); + return { + handlers, calls, errors, resolveInit, rejectInit, + setDB(value) { database = value; }, + async request(route) { + const response = { statusCode: 200, body: null, + status(code) { this.statusCode = code; return this; }, + json(body) { this.body = body; return this; } }; + await handlers.get(route)({}, response); + return response; + } + }; +} + +const flush = () => new Promise(resolve => setImmediate(resolve)); + +test('import neither initializes the database nor opens a listener', () => { + assert.deepEqual(load().calls, []); +}); + +test('main waits for initialization before listening', async () => { + const fixture = load({ main: true }); + assert.deepEqual(fixture.calls, [['init']]); + fixture.resolveInit(); + await flush(); + assert.deepEqual(fixture.calls, [['init'], ['listen', 3001], ['ref']]); +}); + +test('initialization rejection is reported and never starts a listener', async () => { + const fixture = load({ main: true }); + const error = new Error('synthetic initialization failure'); + fixture.rejectInit(error); + await flush(); + assert.deepEqual(fixture.calls, [['init']]); + assert.deepEqual(fixture.errors, [error]); +}); + +test('health is liveness, but readiness fails before database initialization', async () => { + const fixture = load(); + assert.equal((await fixture.request('/health')).statusCode, 200); + const response = await fixture.request('/ready'); + assert.equal(response.statusCode, 503); + assert.equal(response.body.ready, false); +}); + +test('readiness waits for a successful database query', async () => { + const fixture = load(); + let finish; + fixture.setDB({ get(sql, callback) { + assert.equal(sql, 'SELECT 1'); + finish = callback; + } }); + let settled = false; + const pending = fixture.request('/ready').then(response => { settled = true; return response; }); + await flush(); + assert.equal(settled, false); + finish(null, { '1': 1 }); + const response = await pending; + assert.equal(response.statusCode, 200); + assert.equal(response.body.ready, true); + assert.equal(response.body.db, path.join(__dirname, '..', 'data', 'tpc.db')); +}); + +test('readiness becomes unavailable on a database query failure', async () => { + const fixture = load(); + fixture.setDB({ get(sql, callback) { callback(new Error('synthetic closed database')); } }); + const response = await fixture.request('/ready'); + assert.equal(response.statusCode, 503); + assert.equal(response.body.ready, false); +}); diff --git a/server.js b/server.js index ed25298..ce4c8e1 100644 --- a/server.js +++ b/server.js @@ -4,7 +4,7 @@ const path = require('path'); const PORT = parseInt(process.env.PORT || '3001', 10); // Import DB module -const { initGlobalDB, cleanDB: globalCleanDB } = require('./db/database.js'); +const { initGlobalDB, getDB, cleanDB: globalCleanDB } = require('./db/database.js'); // Import route modules const plansRouter = require('./routes/plans.js'); @@ -44,8 +44,12 @@ globalApp.get('/health', (req, res) => { res.json({ status: 'ok', service: 'tpc-server', timestamp: new Date().toISOString() }); }); -globalApp.get('/ready', (req, res) => { +globalApp.get('/ready', async (req, res) => { try { + const db = getDB(); + await new Promise((resolve, reject) => { + db.get('SELECT 1', (err) => err ? reject(err) : resolve()); + }); const dbPath = path.join(__dirname, 'data', 'tpc.db'); res.json({ ready: true, db: dbPath }); } catch (err) { From cdf38a4a102a133676f9b8d82b17f3339df7fe8d Mon Sep 17 00:00:00 2001 From: Engineer Date: Mon, 26 Oct 2026 13:10:43 -0400 Subject: [PATCH 2/7] test: validate readiness with real offline Express and SQLite Co-Authored-By: Paperclip --- docs/readiness-runtime-validation.md | 56 +++++++++++++++ package.json | 1 + scripts/readiness-runtime.test.cjs | 104 +++++++++++++++++++++++++++ 3 files changed, 161 insertions(+) create mode 100644 docs/readiness-runtime-validation.md create mode 100644 scripts/readiness-runtime.test.cjs diff --git a/docs/readiness-runtime-validation.md b/docs/readiness-runtime-validation.md new file mode 100644 index 0000000..d770d0f --- /dev/null +++ b/docs/readiness-runtime-validation.md @@ -0,0 +1,56 @@ +# Offline readiness runtime validation + +This complements (does not replace) the six injected-boundary startup contract tests. +It uses actual Express routing, the actual database module/migrations and native +sqlite3 callback API. It never runs the default main entry point or calls providers. +No production module or app API changes are needed. + +## Run + +Install the locked dependencies with lifecycle scripts disabled, then provision only +sqlite3's inspected native binding (package-supported N-API prebuilt or a bounded +source build). Do not run broad dependency lifecycle scripts. Use Node 22 or later +compatible with the lockfile. Other native optional dependencies are not needed. + +Choose private, synthetic absolute output paths and run in an isolated environment: + +```sh +env -i PATH="/path/to/node/bin:/usr/bin:/bin" \ + HOME="$OUTPUT/home" TMPDIR="$OUTPUT/tmp" npm_config_cache="$OUTPUT/cache" \ + NODE_ENV=test TPC_RUNTIME_FIXTURE_ROOT="$OUTPUT/fixtures" \ + timeout 30 node --test scripts/readiness-runtime.test.cjs scripts/startup-contract.test.cjs +``` + +Create home/tmp/cache first. `TPC_RUNTIME_FIXTURE_ROOT` is a **test harness** setting, +not a production database override. The database module has no supported data/path +environment override. `initGlobalDB` uses its module-relative `data/tpc.db`; `initDB` +only publishes the global handle when passed that exact path. Therefore the test +copies only server.js, db/, routes/ and middleware/ byte-for-byte into a fresh +fixture under the specified output, with a dependency symlink to this checkout's +node_modules. It does not copy existing data, user JSON, tracked DBs or public assets. +The copied server is required, never executed as main. Route imports create routers +but do not initialize DBs, contact providers, or start listeners. + +`initGlobalDB(true)` performs real schema/index/FTS migration while skipping JSON +imports. NODE_ENV=test suppresses backup creation. The test uses a real synthetic +on-disk DB, not a mock or a misleading `:memory:` global handle. It checks: + +- uninitialized `/ready`: 503 with existing error shape; +- `/health` before initialization: independent 200; +- migrated database and real SELECT 1 callback: 200 with existing path response; +- closed native handle: real SQLITE_MISUSE callback yields 503, health still 200. + +The only listener binds **127.0.0.1, ephemeral port 0** and is closed in finally; +requests have a two-second timeout. The DB is closed, the fixture recursively removed, +and removal asserted even after test failures. Use an outer finite process timeout +as shown for protection against native initialization stalls. Run separate processes +to obtain fresh module-global DB state. + +The existing default Jest suites are not claimed here. They use Supertest's default +app binding (not this explicit loopback listener), and the stdio MCP contract spawns +a separate MCP process. Running those unchanged exceeds this validation's listener/ +MCP containment. Playwright/e2e and performance/provider coverage are also excluded. +No Windows or hosted CI evidence is implied. Authorized root should arrange independent +review of the exact committed test diff and, if accepted, public-repository Windows +Actions execution under existing ownership/approval gates. No remote write is needed +for this local validation leaf. diff --git a/package.json b/package.json index 45fdbe8..1e65f90 100644 --- a/package.json +++ b/package.json @@ -6,6 +6,7 @@ "test": "npm run test:unit", "test:unit": "cross-env NODE_ENV=test jest --runInBand", "test:startup-contract": "node --test scripts/startup-contract.test.cjs", + "test:readiness-runtime": "node --test scripts/readiness-runtime.test.cjs", "test:e2e": "playwright test", "mcp": "node mcp-server.js", "smoke:mcp": "node scripts/mcp-smoke.js", diff --git a/scripts/readiness-runtime.test.cjs b/scripts/readiness-runtime.test.cjs new file mode 100644 index 0000000..8994e6b --- /dev/null +++ b/scripts/readiness-runtime.test.cjs @@ -0,0 +1,104 @@ +'use strict'; + +// Real modules, copied byte-for-byte to isolate database.js's __dirname paths. +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs/promises'); +const path = require('node:path'); +const http = require('node:http'); +const { once } = require('node:events'); + +const source = path.resolve(__dirname, '..'); +const root = process.env.TPC_RUNTIME_FIXTURE_ROOT; +if (!root || !path.isAbsolute(root) || process.env.NODE_ENV !== 'test') { + throw new Error('Set NODE_ENV=test and absolute TPC_RUNTIME_FIXTURE_ROOT to a private synthetic output directory'); +} + +function request(server, route) { + return new Promise((resolve, reject) => { + const req = http.get({ host: '127.0.0.1', port: server.address().port, path: route }, res => { + let body = ''; + res.setEncoding('utf8'); + res.on('data', chunk => { body += chunk; }); + res.on('error', reject); + res.on('end', () => { + try { resolve({ status: res.statusCode, body: JSON.parse(body) }); } + catch (error) { reject(error); } + }); + }); + req.setTimeout(2000, () => req.destroy(new Error('Request timed out'))); + req.on('error', reject); + }); +} + +function closeDb(db) { + return new Promise((resolve, reject) => db.close(error => error ? reject(error) : resolve())); +} + +test('real Express readiness with real sqlite3 and synthetic on-disk database', { timeout: 15000 }, async t => { + await fs.mkdir(root, { recursive: true }); + const fixture = await fs.mkdtemp(path.join(root, 'ready-')); + let server; + let db; + try { + // Do not copy tracked data, user JSON, DBs, public assets, or main-entry scripts. + for (const entry of ['server.js', 'db', 'routes', 'middleware']) { + await fs.cp(path.join(source, entry), path.join(fixture, entry), { recursive: true }); + } + await fs.symlink(path.join(source, 'node_modules'), path.join(fixture, 'node_modules'), 'junction'); + const database = require(path.join(fixture, 'db/database.js')); + const { app } = require(path.join(fixture, 'server.js')); + assert.throws(() => database.getDB(), /not initialized/); + server = http.createServer(app); + server.listen(0, '127.0.0.1'); + await once(server, 'listening'); + + await t.test('uninitialized database returns 503', async () => { + const response = await request(server, '/ready'); + assert.equal(response.status, 503); + assert.deepEqual(response.body, { ready: false, error: 'Global DB not initialized' }); + }); + await t.test('health is independent before initialization', async () => { + const response = await request(server, '/health'); + assert.equal(response.status, 200); + assert.equal(response.body.status, 'ok'); + assert.equal(response.body.service, 'tpc-server'); + assert.ok(Number.isFinite(Date.parse(response.body.timestamp))); + }); + await t.test('real migration and successful SQLite callback return 200', async () => { + // skipMigration skips JSON imports, NOT the actual schema/FTS migrations. + await database.initGlobalDB(true); + db = database.getDB(); + assert.equal(db.constructor.name, 'Database'); + const response = await request(server, '/ready'); + assert.equal(response.status, 200); + assert.deepEqual(response.body, { ready: true, db: path.join(fixture, 'data/tpc.db') }); + assert.ok((await fs.stat(path.join(fixture, 'data/tpc.db'))).size > 0); + assert.deepEqual(await database.getAll(db, 'plans'), []); + assert.deepEqual(await database.getAll(db, 'thoughts'), []); + }); + await t.test('closed native handle returns 503 while health stays 200', async () => { + await closeDb(db); + db = undefined; + const response = await request(server, '/ready'); + assert.equal(response.status, 503); + assert.equal(response.body.ready, false); + assert.match(response.body.error, /SQLITE_MISUSE.*Database is closed/); + assert.equal((await request(server, '/health')).status, 200); + }); + } finally { + try { + if (server) { + server.closeAllConnections(); + await new Promise((resolve, reject) => server.close(error => error ? reject(error) : resolve())); + } + } finally { + try { + if (db) await closeDb(db); + } finally { + await fs.rm(fixture, { recursive: true, force: true }); + await assert.rejects(fs.stat(fixture), { code: 'ENOENT' }); + } + } + } +}); From 534e09ab84cad33e3371c934536a5ab652889d9c Mon Sep 17 00:00:00 2001 From: Engineer Date: Mon, 26 Oct 2026 14:09:14 -0400 Subject: [PATCH 3/7] ci: prepare bounded native readiness qualification on Ubuntu and Windows Co-Authored-By: Paperclip --- .github/workflows/readiness.yml | 59 ++++++++++++++++++++++++ docs/readiness-platform-ci.md | 82 +++++++++++++++++++++++++++++++++ scripts/readiness-ci.cjs | 71 ++++++++++++++++++++++++++++ scripts/readiness-ci.test.cjs | 46 ++++++++++++++++++ 4 files changed, 258 insertions(+) create mode 100644 .github/workflows/readiness.yml create mode 100644 docs/readiness-platform-ci.md create mode 100644 scripts/readiness-ci.cjs create mode 100644 scripts/readiness-ci.test.cjs diff --git a/.github/workflows/readiness.yml b/.github/workflows/readiness.yml new file mode 100644 index 0000000..8b53d0e --- /dev/null +++ b/.github/workflows/readiness.yml @@ -0,0 +1,59 @@ +name: Bounded readiness qualification + +on: + workflow_dispatch: + pull_request: + branches: [main, master] + push: + branches: [main, master] + +permissions: + contents: read + +jobs: + readiness: + name: Readiness / ${{ matrix.os }} / Node 22 + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} + timeout-minutes: 8 + defaults: + run: + shell: bash + env: + npm_config_cache: ${{ runner.temp }}/readiness-cache + HOME: ${{ runner.temp }}/readiness-home + TMPDIR: ${{ runner.temp }} + TMP: ${{ runner.temp }} + TEMP: ${{ runner.temp }} + TPC_RUNTIME_FIXTURE_ROOT: ${{ runner.temp }}/readiness-probes + READINESS_FIXTURE_PARENT: ${{ runner.temp }}/readiness-qualification + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + - uses: actions/setup-node@v4 + with: + node-version: 22 + - name: Locked dependencies without lifecycle scripts + run: npm ci --ignore-scripts --no-audit --no-fund --fetch-retries=0 --fetch-timeout=20000 + - name: Provision only sqlite3 via inspected package-supported N-API route + # No source-build fallback or other native packages. Missing prebuilt fails closed. + run: node -e 'const p=require("node:path");const r=require("node:child_process").spawnSync(process.execPath,[require.resolve("prebuild-install/bin.js"),"-r","napi"],{cwd:p.dirname(require.resolve("sqlite3/package.json")),stdio:"inherit",timeout:60000,killSignal:"SIGKILL"});process.exit(r.error||r.signal||r.status===null?1:r.status)' + - name: Offline runner regression probes + run: node --test --test-timeout=10000 scripts/readiness-ci.test.cjs + - name: Actual unchanged readiness contracts and native integration + # Write only sanitized runner output, not install logs, DBs or native files. + run: | + mkdir -p readiness-results + node scripts/readiness-ci.cjs "$READINESS_FIXTURE_PARENT" > readiness-results/readiness.log 2>&1 + - name: Upload concise readiness result only + if: always() + uses: actions/upload-artifact@v4 + with: + name: readiness-${{ matrix.os }}-node22 + path: readiness-results/readiness.log + if-no-files-found: ignore + retention-days: 7 diff --git a/docs/readiness-platform-ci.md b/docs/readiness-platform-ci.md new file mode 100644 index 0000000..b5bec8e --- /dev/null +++ b/docs/readiness-platform-ci.md @@ -0,0 +1,82 @@ +# Bounded readiness qualification (candidate, not platform acceptance) + +`.github/workflows/readiness.yml` is separate from existing CI, which is unchanged. +It proposes Ubuntu and native Windows hosted runners on Node 22, with finite +8-minute jobs. No secrets, deployments, write permissions, `pull_request_target`, +readiness skips or `continue-on-error` are used. Checkout does not retain credentials. + +## Installation boundary + +`npm ci --ignore-scripts` honors the existing lockfile, with lifecycle scripts +(including other native dependencies and browser downloads) disabled. The only +explicit native provisioning is the installed `prebuild-install/bin.js -r napi`, +run in the installed sqlite3 package directory with a 60-second deadline. This is +the first branch of sqlite3 5.1.7's inspected install script. It selects that +package/version's upstream N-API prebuilt; there is no source-build fallback, +ad-hoc executable, broad rebuild, or local binary copied into CI. Failure is a +qualification failure, not permission to try unrelated packaging repairs. + +This upstream native asset is version-selected by the package but not independently +checksum-pinned by package-lock.json. The runtime log records the loaded native +SHA256, Node/OS/arch, Express, sqlite3 and SQLite versions. This is provenance, +not independent supply-chain authentication. Existing local author receipts show +a source build failed because Python lacked distutils, then this supported N-API +route worked on Linux. That does not prove either hosted download will succeed. + +## Local isolated execution + +Use an absolute, private synthetic output directory, not a source/user data path: + +```sh +env -i PATH=/opt/node-runtime/bin:/usr/bin:/bin \ + HOME="$O/home" TMPDIR="$O/tmp" npm_config_cache="$O/cache" \ + TPC_RUNTIME_FIXTURE_ROOT="$O/fixtures" \ + timeout 35 node scripts/readiness-ci.cjs "$O/fixtures" + +env -i PATH=/opt/node-runtime/bin:/usr/bin:/bin \ + HOME="$O/home" TMPDIR="$O/tmp" npm_config_cache="$O/cache" \ + TPC_RUNTIME_FIXTURE_ROOT="$O/fixtures" \ + timeout 15 node --test --test-timeout=10000 scripts/readiness-ci.test.cjs +``` + +The runner creates a unique synthetic root and a minimal child environment, +including Windows system/path variables when present. It never inherits +NODE_OPTIONS/NODE_PATH, credentials or CI/proxy variables. It runs only the two +existing test files, byte-for-byte unchanged, in a Node child with a 25-second +process deadline and 1 MiB output bound. Nonzero exits propagate; signals, +spawn/output errors and deadlines fail closed. Parent finally removes its entire +synthetic tree even after killing the child. This is not process-tree containment +for arbitrary subprocesses, and hard termination of the parent cannot guarantee +cleanup. Neither test starts MCP/provider children or the main server listener. + +The original integration harness uses a Windows-compatible directory junction, +module-relative synthetic files, native SQLite and 127.0.0.1 ephemeral HTTP only. +The six startup contracts and four native integration states are not weakened. +The probe file tests allowlist behavior, actual child isolation, exit 7, a stalled +child deadline, invalid relative parent rejection and success/failure cleanup. + +## Authorized-root handoff and acceptance + +Only after independent review and authorization, root may push the supplied +branch or integrate its exact commit. This worker has not pushed or dispatched +Actions. If using a non-default branch, ensure the workflow exists on the default +branch before attempting manual `workflow_dispatch`, or use an authorized PR. +Inspect the actual run and both distinct matrix jobs, not only workflow syntax: + +1. Confirm remote head equals the reviewed candidate SHA. +2. Confirm locked install, sqlite3 provisioning, runner probes and native runtime + steps all succeed on **both** Ubuntu and Windows without skips. +3. Download `readiness-ubuntu-latest-node22` and + `readiness-windows-latest-node22`. Inspect native provenance, 11 pass/0 fail, + no skips, uninitialized 503, initialized 200, closed-handle 503 and independent + health 200. Confirm synthetic root removal and compare exact tested SHA. +4. If setup fails, inspect the Actions step output; no install log is uploaded. + An absent runtime artifact is not success. Artifacts contain only the sanitized + bounded test/provenance log (synthetic paths replaced), never DBs, binaries, + source/user data or credentials, and expire after seven days. + +Windows path/case-insensitive environment handling and junction support are +static reasoning until the actual native Windows job succeeds. Local Linux +verification is not Windows, hosted-CI, service, deployment or broad Jest proof. +All existing approval/publication/ownership gates remain. Rollback is revert the +workflow/runner/docs commit; production and dependency files are unchanged. diff --git a/scripts/readiness-ci.cjs b/scripts/readiness-ci.cjs new file mode 100644 index 0000000..6e1e56a --- /dev/null +++ b/scripts/readiness-ci.cjs @@ -0,0 +1,71 @@ +'use strict'; + +// Deliberately only a bounded Node child; not a general command runner. +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); +const { spawnSync } = require('node:child_process'); +const { createHash } = require('node:crypto'); + +function childEnvironment(root, inherited = process.env) { + const env = { NODE_ENV: 'test', HOME: path.join(root, 'home'), + USERPROFILE: path.join(root, 'home'), TMPDIR: path.join(root, 'tmp'), + TMP: path.join(root, 'tmp'), TEMP: path.join(root, 'tmp'), + npm_config_cache: path.join(root, 'cache'), + TPC_RUNTIME_FIXTURE_ROOT: path.join(root, 'fixtures') }; + // Windows variable names are case insensitive. Never forward NODE_OPTIONS, + // NODE_PATH, credentials, proxy settings or the ambient CI environment. + for (const name of ['PATH', 'SystemRoot', 'WINDIR', 'ComSpec', 'PATHEXT']) { + const key = Object.keys(inherited).find(k => k.toLowerCase() === name.toLowerCase()); + if (key) env[name] = inherited[key]; + } + return env; +} + +function runBounded(args, parent, timeout = 25000) { + if (!parent || !path.isAbsolute(parent)) throw new Error('An absolute synthetic parent directory is required'); + fs.mkdirSync(parent, { recursive: true }); + const root = fs.mkdtempSync(path.join(parent, 'readiness-ci-')); + try { + const env = childEnvironment(root); + for (const key of ['HOME', 'TMPDIR', 'npm_config_cache', 'TPC_RUNTIME_FIXTURE_ROOT']) { + fs.mkdirSync(env[key], { recursive: true }); + } + const result = spawnSync(process.execPath, args, { + cwd: path.resolve(__dirname, '..'), env, timeout, killSignal: 'SIGKILL', + encoding: 'utf8', maxBuffer: 1024 * 1024, windowsHide: true + }); + const sanitize = value => (value || '').split(root).join('[synthetic-root]'); + const output = sanitize(result.stdout) + sanitize(result.stderr); + return { status: result.error || result.signal || result.status === null ? 1 : result.status, + reason: result.error ? result.error.code : result.signal || 'exit', output }; + } finally { + fs.rmSync(root, { recursive: true, force: true }); + if (fs.existsSync(root)) throw new Error('Synthetic root cleanup failed'); + console.log('Synthetic root removed: true'); + } +} + +function provenance() { + const sqlite = require('sqlite3'); + const native = path.join(path.dirname(require.resolve('sqlite3/package.json')), 'build/Release/node_sqlite3.node'); + console.log(JSON.stringify({ node: process.version, platform: process.platform, arch: process.arch, + os: os.release(), express: require('express/package.json').version, + sqlite3: require('sqlite3/package.json').version, sqlite: sqlite.VERSION, + nativeSHA256: createHash('sha256').update(fs.readFileSync(native)).digest('hex') })); +} + +if (require.main === module) { + try { + provenance(); + const result = runBounded(['--test', 'scripts/startup-contract.test.cjs', + 'scripts/readiness-runtime.test.cjs'], process.argv[2]); + process.stdout.write(result.output); + console.log(`Bounded child: ${result.reason}; exit=${result.status}`); + process.exitCode = result.status; + } catch (error) { + console.error(`Readiness runner failed: ${error.code || error.message}`); + process.exitCode = 1; + } +} +module.exports = { childEnvironment, runBounded }; diff --git a/scripts/readiness-ci.test.cjs b/scripts/readiness-ci.test.cjs new file mode 100644 index 0000000..d3cce0d --- /dev/null +++ b/scripts/readiness-ci.test.cjs @@ -0,0 +1,46 @@ +'use strict'; +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { childEnvironment, runBounded } = require('./readiness-ci.cjs'); + +const parent = process.env.TPC_RUNTIME_FIXTURE_ROOT; +if (!parent || !path.isAbsolute(parent)) throw new Error('Absolute synthetic fixture parent required'); + +test('allowlist drops ambient secrets and Node injection, preserves Windows system variables', () => { + const env = childEnvironment(parent, { Path: 'synthetic-path', SYSTEMROOT: 'C:\\Windows', + WINDIR: 'C:\\Windows', ComSpec: 'cmd.exe', PATHEXT: '.EXE', + NODE_OPTIONS: '--require evil', NODE_PATH: 'evil', SECRET: 'sentinel' }); + assert.equal(env.PATH, 'synthetic-path'); + assert.equal(env.SystemRoot, 'C:\\Windows'); + assert.equal(env.NODE_OPTIONS, undefined); + assert.equal(env.NODE_PATH, undefined); + assert.equal(env.SECRET, undefined); +}); + +test('bounded child isolates environment, propagates real nonzero, times out and cleans up', () => { + fs.mkdirSync(parent, { recursive: true }); + const before = fs.readdirSync(parent).sort(); + process.env.READINESS_PROBE_SENTINEL = 'must-not-inherit'; + try { + const ok = runBounded(['-e', `const a=require('node:assert/strict'); + a.equal(process.env.READINESS_PROBE_SENTINEL,undefined); + a.equal(process.env.NODE_ENV,'test'); + require('node:fs').writeFileSync(require('node:path').join(process.env.TPC_RUNTIME_FIXTURE_ROOT,'synthetic.txt'),'fixture');`], parent); + assert.equal(ok.status, 0); + const fail = runBounded(['-e', 'process.exit(7)'], parent); + assert.equal(fail.status, 7); + const stalled = runBounded(['-e', 'setInterval(()=>{},1000)'], parent, 200); + assert.equal(stalled.status, 1); + assert.equal(stalled.reason, 'ETIMEDOUT'); + assert.deepEqual(fs.readdirSync(parent).sort(), before); + console.log('Expected negative probes: exit=7, ETIMEDOUT; all roots removed'); + } finally { + delete process.env.READINESS_PROBE_SENTINEL; + } +}); + +test('relative fixture parents fail closed', () => { + assert.throws(() => runBounded(['-e', 'process.exit(0)'], 'relative'), /absolute/); +}); From 144f7962b46236f8bdb93375d25c9ea7b8ec2246 Mon Sep 17 00:00:00 2001 From: Engineer Date: Mon, 26 Oct 2026 15:09:07 -0400 Subject: [PATCH 4/7] fix: redact escaped synthetic roots in readiness diagnostics Bound literal root representations and retain unrelated output and runner failure semantics. Co-Authored-By: Paperclip --- docs/readiness-platform-ci.md | 14 +++++++-- scripts/readiness-ci.cjs | 19 ++++++++++-- scripts/readiness-ci.test.cjs | 54 ++++++++++++++++++++++++++++++++++- 3 files changed, 82 insertions(+), 5 deletions(-) diff --git a/docs/readiness-platform-ci.md b/docs/readiness-platform-ci.md index b5bec8e..82e9f28 100644 --- a/docs/readiness-platform-ci.md +++ b/docs/readiness-platform-ci.md @@ -54,6 +54,8 @@ module-relative synthetic files, native SQLite and 127.0.0.1 ephemeral HTTP only The six startup contracts and four native integration states are not weakened. The probe file tests allowlist behavior, actual child isolation, exit 7, a stalled child deadline, invalid relative parent rejection and success/failure cleanup. +Additional regressions cover output overflow and signals, plus real bounded child +stdout/stderr diagnostic redaction (including a spaces/Unicode synthetic parent). ## Authorized-root handoff and acceptance @@ -72,8 +74,16 @@ Inspect the actual run and both distinct matrix jobs, not only workflow syntax: health 200. Confirm synthetic root removal and compare exact tested SHA. 4. If setup fails, inspect the Actions step output; no install log is uploaded. An absent runtime artifact is not success. Artifacts contain only the sanitized - bounded test/provenance log (synthetic paths replaced), never DBs, binaries, - source/user data or credentials, and expire after seven days. + bounded test/provenance log, not DBs or binaries, and expire after seven days. + The runner redacts its known synthetic root in raw and forward-slash-normalized + forms, each also with doubled backslashes (Node inspection) or one JSON string + escaping layer. Descendant suffixes and unrelated diagnostic text are retained; + path-prefix/sibling lookalikes are not replaced. This is a finite literal-root + diagnostic filter, **not arbitrary secret or user-data scrubbing**. It does not + promise case-folded paths, mixed separator spellings, URL/HTML encodings, + repeated JSON encoding, truncated roots, or every inspection quoting/control + character convention. Input must remain synthetic and environment-allowlisted; + never upload arbitrary program output on the assumption this filter makes it safe. Windows path/case-insensitive environment handling and junction support are static reasoning until the actual native Windows job succeeds. Local Linux diff --git a/scripts/readiness-ci.cjs b/scripts/readiness-ci.cjs index 6e1e56a..c41e681 100644 --- a/scripts/readiness-ci.cjs +++ b/scripts/readiness-ci.cjs @@ -22,6 +22,21 @@ function childEnvironment(root, inherited = process.env) { return env; } +// Finite representations of this runner's known root, not a secret scrubber. +function redactSyntheticRoot(value, root) { + const forms = new Set(); + for (const form of [root, root.replaceAll('\\', '/')]) { + forms.add(form); + forms.add(form.replaceAll('\\', '\\\\')); // Node inspection backslashes + forms.add(JSON.stringify(form).slice(1, -1)); + } + const alternatives = [...forms].sort((a, b) => b.length - a.length) + .map(form => form.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('|'); + // Do not match a sibling sharing the root's prefix or a longer path prefix. + const pattern = new RegExp(`(^|[^a-zA-Z0-9_./\\\\-])(?:${alternatives})(?=$|[/\\\\\\s'"\x60,;:)\\]}])`, 'g'); + return (value || '').replace(pattern, '$1[synthetic-root]'); +} + function runBounded(args, parent, timeout = 25000) { if (!parent || !path.isAbsolute(parent)) throw new Error('An absolute synthetic parent directory is required'); fs.mkdirSync(parent, { recursive: true }); @@ -35,7 +50,7 @@ function runBounded(args, parent, timeout = 25000) { cwd: path.resolve(__dirname, '..'), env, timeout, killSignal: 'SIGKILL', encoding: 'utf8', maxBuffer: 1024 * 1024, windowsHide: true }); - const sanitize = value => (value || '').split(root).join('[synthetic-root]'); + const sanitize = value => redactSyntheticRoot(value, root); const output = sanitize(result.stdout) + sanitize(result.stderr); return { status: result.error || result.signal || result.status === null ? 1 : result.status, reason: result.error ? result.error.code : result.signal || 'exit', output }; @@ -68,4 +83,4 @@ if (require.main === module) { process.exitCode = 1; } } -module.exports = { childEnvironment, runBounded }; +module.exports = { childEnvironment, runBounded, redactSyntheticRoot }; diff --git a/scripts/readiness-ci.test.cjs b/scripts/readiness-ci.test.cjs index d3cce0d..9850f22 100644 --- a/scripts/readiness-ci.test.cjs +++ b/scripts/readiness-ci.test.cjs @@ -3,7 +3,7 @@ const { test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { childEnvironment, runBounded } = require('./readiness-ci.cjs'); +const { childEnvironment, runBounded, redactSyntheticRoot } = require('./readiness-ci.cjs'); const parent = process.env.TPC_RUNTIME_FIXTURE_ROOT; if (!parent || !path.isAbsolute(parent)) throw new Error('Absolute synthetic fixture parent required'); @@ -41,6 +41,58 @@ test('bounded child isolates environment, propagates real nonzero, times out and } }); +test('synthetic Windows roots redact raw, JSON, inspection and normalized forms only', () => { + const { inspect } = require('node:util'); + for (const root of ['D:\\a\\_temp\\readiness-ci-ABC123', + 'C:\\space dir\\雪 café\\readiness-ci-DEF456', + 'E:\\dots.[x]+(y)\\readiness-ci-GHI789']) { + const normalized = root.replaceAll('\\', '/'); + for (const form of [root, normalized]) { + for (const render of [value => value, JSON.stringify, inspect]) { + for (const suffix of ['', '\\fixtures', '/fixtures']) { + const input = `prefix ${render(form + suffix)} suffix`; + assert.equal(redactSyntheticRoot(input, root), + `prefix ${render('[synthetic-root]' + suffix)} suffix`); + } + } + } + const unrelated = `other C:\\unrelated\\file ${root}-sibling ${root}Extra prefix${root}`; + assert.equal(redactSyntheticRoot(unrelated, root), unrelated); + assert.equal(redactSyntheticRoot(`(${root}) ${root}/child`, root), + '([synthetic-root]) [synthetic-root]/child'); + } +}); + +test('actual child stdout/stderr representations are redacted without hiding failure', () => { + const before = fs.readdirSync(parent).sort(); + const spacedParent = path.join(parent, 'space 雪 café'); + const result = runBounded(['-e', `const path=require('node:path'); + const root=path.dirname(process.env.TPC_RUNTIME_FIXTURE_ROOT); + process.stdout.write(JSON.stringify({root})+'\\n'); + process.stderr.write(require('node:util').inspect({root})+'\\n'); + process.stderr.write(root+'/fixtures sentinel-unrelated'); process.exit(9);`], spacedParent); + assert.equal(result.status, 9); + assert.equal(result.reason, 'exit'); + assert.equal(result.output.includes(parent), false); + assert.equal(result.output.split('[synthetic-root]').length - 1, 3); + assert.match(result.output, /sentinel-unrelated/); + assert.deepEqual(fs.readdirSync(spacedParent), []); + fs.rmdirSync(spacedParent); + assert.deepEqual(fs.readdirSync(parent).sort(), before); +}); + +test('overflow and signal fail closed and clean up', () => { + const before = fs.readdirSync(parent).sort(); + const overflow = runBounded(['-e', 'process.stdout.write("x".repeat(2*1024*1024))'], parent, 2000); + assert.equal(overflow.status, 1); + assert.equal(overflow.reason, 'ENOBUFS'); + const signal = runBounded(['-e', 'process.kill(process.pid,"SIGTERM")'], parent, 2000); + assert.equal(signal.status, 1); + if (process.platform !== 'win32') assert.equal(signal.reason, 'SIGTERM'); + // Windows signal reporting is platform-dependent; status must still fail closed. + assert.deepEqual(fs.readdirSync(parent).sort(), before); +}); + test('relative fixture parents fail closed', () => { assert.throws(() => runBounded(['-e', 'process.exit(0)'], 'relative'), /absolute/); }); From 27a2c6dd5ee3f58342c1e27438c2f2f22beff18f Mon Sep 17 00:00:00 2001 From: Engineer Date: Mon, 26 Oct 2026 16:07:51 -0400 Subject: [PATCH 5/7] fix: redact known synthetic roots at ANSI SGR boundaries Recognize finite numeric/semicolon SGR framing without stripping controls or changing path lookalike handling. Add baseline-failing Windows-string and actual nonzero child stdout/stderr regressions. Co-Authored-By: Paperclip --- docs/readiness-platform-ci.md | 12 ++++++++++-- scripts/readiness-ci.cjs | 5 ++++- scripts/readiness-ci.test.cjs | 37 +++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/docs/readiness-platform-ci.md b/docs/readiness-platform-ci.md index 82e9f28..b072f9c 100644 --- a/docs/readiness-platform-ci.md +++ b/docs/readiness-platform-ci.md @@ -56,6 +56,12 @@ The probe file tests allowlist behavior, actual child isolation, exit 7, a stall child deadline, invalid relative parent rejection and success/failure cleanup. Additional regressions cover output overflow and signals, plus real bounded child stdout/stderr diagnostic redaction (including a spaces/Unicode synthetic parent). +ANSI regressions cover static Windows roots and actual child stdout/stderr with +exit 13 and cleanup; these fail against the previous sanitizer. Numeric/semicolon +SGR sequences (`ESC [` followed by zero or more digits/semicolons and `m`) are +recognized only as root boundaries, retained byte-for-byte in output. This narrow +correction avoids treating the final `m` of a color prefix or the ESC of a reset +as a path character; it does not strip colors or change subprocess handling. ## Authorized-root handoff and acceptance @@ -82,8 +88,10 @@ Inspect the actual run and both distinct matrix jobs, not only workflow syntax: diagnostic filter, **not arbitrary secret or user-data scrubbing**. It does not promise case-folded paths, mixed separator spellings, URL/HTML encodings, repeated JSON encoding, truncated roots, or every inspection quoting/control - character convention. Input must remain synthetic and environment-allowlisted; - never upload arbitrary program output on the assumption this filter makes it safe. + character convention. Other terminal sequences (including non-SGR CSI, OSC, + colon-form SGR and incomplete/malformed escapes) are not recognized framing. + Input must remain synthetic and environment-allowlisted; never upload arbitrary + program output on the assumption this filter makes it safe. Windows path/case-insensitive environment handling and junction support are static reasoning until the actual native Windows job succeeds. Local Linux diff --git a/scripts/readiness-ci.cjs b/scripts/readiness-ci.cjs index c41e681..f2acd7f 100644 --- a/scripts/readiness-ci.cjs +++ b/scripts/readiness-ci.cjs @@ -32,8 +32,11 @@ function redactSyntheticRoot(value, root) { } const alternatives = [...forms].sort((a, b) => b.length - a.length) .map(form => form.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('|'); + // Numeric/semicolon SGR is diagnostic framing, not a path continuation. + // Retain it verbatim; do not strip controls or parse other terminal sequences. + const sgr = '\\x1b\\[[0-9;]*m'; // Do not match a sibling sharing the root's prefix or a longer path prefix. - const pattern = new RegExp(`(^|[^a-zA-Z0-9_./\\\\-])(?:${alternatives})(?=$|[/\\\\\\s'"\x60,;:)\\]}])`, 'g'); + const pattern = new RegExp(`(^|${sgr}|[^a-zA-Z0-9_./\\\\-])(?:${alternatives})(?=$|${sgr}|[/\\\\\\s'"\x60,;:)\\]}])`, 'g'); return (value || '').replace(pattern, '$1[synthetic-root]'); } diff --git a/scripts/readiness-ci.test.cjs b/scripts/readiness-ci.test.cjs index 9850f22..8c9d83c 100644 --- a/scripts/readiness-ci.test.cjs +++ b/scripts/readiness-ci.test.cjs @@ -63,6 +63,43 @@ test('synthetic Windows roots redact raw, JSON, inspection and normalized forms } }); +test('ANSI SGR framing redacts Windows roots while retaining framing and lookalikes', () => { + for (const root of ['D:\\a\\_temp\\readiness-ci-ABC123', + 'C:\\space 雪 café\\dots.[x]+(y)\\readiness-ci-DEF456']) { + for (const form of [root, root.replaceAll('\\', '/')]) { + for (const prefix of ['', '\x1b[31m', '\x1b[1;32m', '\x1b[m']) { + const input = prefix + form + '\x1b[0m'; + assert.equal(redactSyntheticRoot(input, root), prefix + '[synthetic-root]\x1b[0m'); + for (const suffix of ['-sibling', 'Extra']) { + const sibling = prefix + form + suffix + '\x1b[0m'; + assert.equal(redactSyntheticRoot(sibling, root), sibling); + } + } + const longerPrefix = '\x1b[31mprefix' + form + '\x1b[0m'; + assert.equal(redactSyntheticRoot(longerPrefix, root), longerPrefix); + } + // This is a finite SGR boundary rule, not an arbitrary terminal parser. + for (const suffix of ['\x1b[2K', '\x1b[bogus', '\x1b']) { + assert.equal(redactSyntheticRoot(root + suffix, root), root + suffix); + } + } +}); + +test('actual ANSI child stdout/stderr redact roots, propagate exit13 and clean up', () => { + fs.mkdirSync(parent, { recursive: true }); + const before = fs.readdirSync(parent).sort(); + const result = runBounded(['-e', `const path=require('node:path'); + const root=path.dirname(process.env.TPC_RUNTIME_FIXTURE_ROOT); + process.stdout.write('stdout: \\x1b[31m'+root+'\\x1b[0m\\n'); + process.stderr.write('stderr: '+root+'\\x1b[0m\\n'); + process.exit(13);`], parent, 2000); + assert.equal(result.status, 13); + assert.equal(result.reason, 'exit'); + assert.equal(result.output.includes(parent), false); + assert.equal(result.output, 'stdout: \x1b[31m[synthetic-root]\x1b[0m\nstderr: [synthetic-root]\x1b[0m\n'); + assert.deepEqual(fs.readdirSync(parent).sort(), before); +}); + test('actual child stdout/stderr representations are redacted without hiding failure', () => { const before = fs.readdirSync(parent).sort(); const spacedParent = path.join(parent, 'space 雪 café'); From cde8bf2dae9dff84fde1dee2f2dfe85f76c38f06 Mon Sep 17 00:00:00 2001 From: Engineer Date: Tue, 27 Oct 2026 12:52:06 -0400 Subject: [PATCH 6/7] fix(ci): initialize runner isolation in step context Move runner.temp expressions out of unsupported job-level env while preserving the exact isolated environment for all existing steps. Co-Authored-By: Paperclip --- .github/workflows/readiness.yml | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/.github/workflows/readiness.yml b/.github/workflows/readiness.yml index 8b53d0e..12a4176 100644 --- a/.github/workflows/readiness.yml +++ b/.github/workflows/readiness.yml @@ -22,15 +22,21 @@ jobs: defaults: run: shell: bash - env: - npm_config_cache: ${{ runner.temp }}/readiness-cache - HOME: ${{ runner.temp }}/readiness-home - TMPDIR: ${{ runner.temp }} - TMP: ${{ runner.temp }} - TEMP: ${{ runner.temp }} - TPC_RUNTIME_FIXTURE_ROOT: ${{ runner.temp }}/readiness-probes - READINESS_FIXTURE_PARENT: ${{ runner.temp }}/readiness-qualification steps: + - name: Initialize isolated runner environment + # runner context is available in step env, not job env. + env: + npm_config_cache: ${{ runner.temp }}/readiness-cache + HOME: ${{ runner.temp }}/readiness-home + TMPDIR: ${{ runner.temp }} + TMP: ${{ runner.temp }} + TEMP: ${{ runner.temp }} + TPC_RUNTIME_FIXTURE_ROOT: ${{ runner.temp }}/readiness-probes + READINESS_FIXTURE_PARENT: ${{ runner.temp }}/readiness-qualification + run: | + for name in npm_config_cache HOME TMPDIR TMP TEMP TPC_RUNTIME_FIXTURE_ROOT READINESS_FIXTURE_PARENT; do + printf '%s=%s\n' "$name" "${!name}" >> "$GITHUB_ENV" + done - uses: actions/checkout@v4 with: persist-credentials: false From 899f5c219a49781f65ffed8cb927fa6ef181038e Mon Sep 17 00:00:00 2001 From: Engineer Date: Tue, 27 Oct 2026 13:07:38 -0400 Subject: [PATCH 7/7] Replace cache timing ratio with deterministic timestamp contracts Co-Authored-By: Paperclip --- docs/example-cache-contract.md | 31 ++++++++++++ routes/exampleTimestampCache.js | 22 +++++++++ routes/tools.js | 14 ++---- tests/test_example_timestamp_cache.js | 68 +++++++++++++++++++++++++++ tests/test_mcp_performance.js | 31 ++---------- 5 files changed, 130 insertions(+), 36 deletions(-) create mode 100644 docs/example-cache-contract.md create mode 100644 routes/exampleTimestampCache.js create mode 100644 tests/test_example_timestamp_cache.js diff --git a/docs/example-cache-contract.md b/docs/example-cache-contract.md new file mode 100644 index 0000000..fcaeb20 --- /dev/null +++ b/docs/example-cache-contract.md @@ -0,0 +1,31 @@ +# Tool example timestamp-cache contract + +`/tools/examples` validates the tool name before accessing a module-wide cache +keyed by the exact tool name. It retains the first timestamp for that key; it +still constructs the examples response on every request. There is no TTL, +expiry, invalidation or whole-response caching guarantee. + +`routes/exampleTimestampCache.js` extracts the existing behavior into a factory +with injectable cache, timestamp clock and delay. Production retains one +module-wide instance (including across `createApp` calls), the same ISO timestamp +payload and the existing artificial blocking delay: 40 ms on a miss, 1 ms on a +hit. Separate factory calls have independent default maps. This is a testability +seam, not a cache redesign or delay optimization. + +The previous cache ratio test ran after earlier tests had requested the same +key, so its purported cold/hot comparison was normally hit/hit. Two tiny +wall-clock HTTP samples cannot establish cache correctness or real-world speed. +The isolated contracts in `tests/test_example_timestamp_cache.js` instead assert +storage, timestamp retention as controlled time advances, distinct-key isolation, +prewarmed hits, factory isolation and exact delay-branch selection. Run without +dependencies using: + +```sh +node --test tests/test_example_timestamp_cache.js +``` + +These contracts also run under the existing Jest test discovery. The performance +suite retains HTTP timestamp-retention coverage; it makes no TTL claim. Other +performance assertions are unchanged. Deterministic branch tests do not measure +or promise real-world speed, throughput or latency. Any such requirement needs +a separately defined controlled benchmark, not a relaxed ratio threshold. diff --git a/routes/exampleTimestampCache.js b/routes/exampleTimestampCache.js new file mode 100644 index 0000000..07aa72e --- /dev/null +++ b/routes/exampleTimestampCache.js @@ -0,0 +1,22 @@ +function sleepMs(ms) { + const start = Date.now(); + while (Date.now() - start < ms) {} +} + +// Timestamp retention and artificial delay selection, not a response cache or TTL. +// Each factory owns fresh state unless a cache is explicitly supplied. +function createExampleTimestampCache({ + cache = new Map(), + now = () => new Date().toISOString(), + delay = sleepMs +} = {}) { + return function getTimestamp(toolName) { + const cacheHit = cache.has(toolName); + const timestamp = cache.get(toolName) || now(); + cache.set(toolName, timestamp); + delay(cacheHit ? 1 : 40); + return timestamp; + }; +} + +module.exports = { createExampleTimestampCache, sleepMs }; diff --git a/routes/tools.js b/routes/tools.js index c79d445..a895352 100644 --- a/routes/tools.js +++ b/routes/tools.js @@ -1,6 +1,7 @@ const express = require('express'); const { Router } = express; const { ToolNotFoundError } = require('../middleware/errorHandler'); +const { createExampleTimestampCache, sleepMs } = require('./exampleTimestampCache'); const router = Router(); @@ -46,12 +47,8 @@ const EXTRA_TOOLS = [ ]; const TOOL_BY_NAME = new Map(TOOL_CATALOG.map(t => [t.tool_name, t])); -const EXAMPLE_TIMESTAMP_CACHE = new Map(); - -function sleepMs(ms) { - const start = Date.now(); - while (Date.now() - start < ms) {} -} +// Preserve the original module-wide, tool-name key scope across app instances. +const getExampleTimestamp = createExampleTimestampCache(); function badRequest(message) { const err = new Error(message); @@ -159,10 +156,7 @@ router.get('/examples', (req, res, next) => { const toolName = req.query.tool_name || 'tool_search_tool_regex_20251119'; const tool = getTool(toolName); - const cacheHit = EXAMPLE_TIMESTAMP_CACHE.has(toolName); - const ts = EXAMPLE_TIMESTAMP_CACHE.get(toolName) || new Date().toISOString(); - EXAMPLE_TIMESTAMP_CACHE.set(toolName, ts); - sleepMs(cacheHit ? 1 : 40); + const ts = getExampleTimestamp(toolName); res.json({ tool_name: toolName, diff --git a/tests/test_example_timestamp_cache.js b/tests/test_example_timestamp_cache.js new file mode 100644 index 0000000..e547b20 --- /dev/null +++ b/tests/test_example_timestamp_cache.js @@ -0,0 +1,68 @@ +const assert = require('node:assert/strict'); +const contractTest = typeof test === 'function' ? test : require('node:test'); +const { createExampleTimestampCache } = require('../routes/exampleTimestampCache'); + +const SEARCH = 'tool_search_tool_regex_20251119'; +const UTILITY = 'core_utility_tools'; + +function fixture(cache = new Map()) { + let time = '2026-01-01T00:00:00.000Z'; + let clockCalls = 0; + const delays = []; + const get = createExampleTimestampCache({ + cache, + now: () => { clockCalls++; return time; }, + delay: ms => delays.push(ms) + }); + return { cache, delays, get, advance: () => { time = '2026-02-01T00:00:00.000Z'; }, + clockCalls: () => clockCalls }; +} + +contractTest('fresh same-key access stores once, retains timestamp and selects cold then hit', () => { + const f = fixture(); + assert.equal(f.cache.has(SEARCH), false); + const first = f.get(SEARCH); + assert.equal(first, '2026-01-01T00:00:00.000Z'); + assert.equal(f.cache.get(SEARCH), first); + assert.deepEqual(f.delays, [40]); + f.advance(); + assert.equal(f.get(SEARCH), first); + assert.equal(f.cache.get(SEARCH), first); + assert.equal(f.clockCalls(), 1); + assert.deepEqual(f.delays, [40, 1]); +}); + +contractTest('distinct valid tool names have independent cold state', () => { + const f = fixture(); + const first = f.get(SEARCH); + f.advance(); + assert.equal(f.cache.has(UTILITY), false); + assert.equal(f.get(UTILITY), '2026-02-01T00:00:00.000Z'); + assert.equal(f.get(SEARCH), first); + assert.equal(f.cache.size, 2); + assert.equal(f.clockCalls(), 2); + assert.deepEqual(f.delays, [40, 40, 1]); +}); + +contractTest('deliberately prewarmed key is a hit, not a presumed cold request', () => { + const timestamp = '2025-12-01T00:00:00.000Z'; + const f = fixture(new Map([[SEARCH, timestamp]])); + f.advance(); + assert.equal(f.get(SEARCH), timestamp); + assert.equal(f.cache.get(SEARCH), timestamp); + assert.equal(f.clockCalls(), 0); + assert.deepEqual(f.delays, [1]); +}); + +contractTest('default factory caches do not leak state between independent instances', () => { + const aDelays = []; + const bDelays = []; + const a = createExampleTimestampCache({ now: () => 'A', delay: ms => aDelays.push(ms) }); + const b = createExampleTimestampCache({ now: () => 'B', delay: ms => bDelays.push(ms) }); + assert.equal(a(SEARCH), 'A'); + assert.equal(b(SEARCH), 'B'); + assert.equal(a(SEARCH), 'A'); + assert.equal(b(SEARCH), 'B'); + assert.deepEqual(aDelays, [40, 1]); + assert.deepEqual(bDelays, [40, 1]); +}); diff --git a/tests/test_mcp_performance.js b/tests/test_mcp_performance.js index e88d6da..953ccb7 100644 --- a/tests/test_mcp_performance.js +++ b/tests/test_mcp_performance.js @@ -179,31 +179,10 @@ describe('MCP Performance Benchmarking Tests', () => { }); }); - describe('Cache Performance Tests', () => { - it('should validate cache efficiency for tool examples', async () => { - // First request - cache miss - const firstStart = Date.now(); - const firstResponse = await testApp - .get('/tools/examples?tool_name=tool_search_tool_regex_20251119') - .expect(200); - const firstTime = Date.now() - firstStart; - - // Second request - cache hit - const secondStart = Date.now(); - const secondResponse = await testApp - .get('/tools/examples?tool_name=tool_search_tool_regex_20251119') - .expect(200); - const secondTime = Date.now() - secondStart; - - // Cache hit should generally be faster, but CI timing jitter can invert tiny samples. - // Keep this as a non-flaky sanity threshold while still catching major regressions. - const speedup = firstTime / Math.max(secondTime, 1); - expect(speedup).toBeGreaterThanOrEqual(0.8); - - console.log(`Cache provides ${speedup.toFixed(2)}x speedup`); - }); - - it('should test cache TTL and invalidation', async () => { + // Cold/hit branch contracts use isolated state and controlled time/delay in + // test_example_timestamp_cache.js, not an end-to-end wall-clock ratio. + describe('Example timestamp retention', () => { + it('should retain the timestamp on repeated HTTP requests', async () => { // Get initial response const initialResponse = await testApp .get('/tools/examples?tool_name=tool_search_tool_regex_20251119') @@ -217,7 +196,7 @@ describe('MCP Performance Benchmarking Tests', () => { .get('/tools/examples?tool_name=tool_search_tool_regex_20251119') .expect(200); - // Should still be cached (TTL is longer) + // This checks retention only; the cache has no TTL/invalidation contract. expect(secondResponse.body.timestamp).toBe(initialTimestamp); }); });