diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ba09526..649ff0f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,7 @@ jobs: typecheck-and-test: name: Typecheck & Test (node 22) runs-on: ubuntu-latest + timeout-minutes: 10 steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 diff --git a/package.json b/package.json index 16b75b1..2c6584b 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "scripts": { "build": "tsc", "typecheck": "tsc --noEmit", - "test": "node --import tsx --test test/*.test.ts", + "test": "node --import tsx --test --experimental-test-isolation=none test/*.test.ts", "start": "node --import tsx src/index.ts", "clean": "rm -rf dist", "version": "node scripts/release-changelog.mjs && git add CHANGELOG.md changelog.d" diff --git a/scripts/release-changelog.d.mts b/scripts/release-changelog.d.mts new file mode 100644 index 0000000..6f556af --- /dev/null +++ b/scripts/release-changelog.d.mts @@ -0,0 +1,2 @@ +export class ReleaseError extends Error {} +export function release(dir: string): string; diff --git a/scripts/release-changelog.mjs b/scripts/release-changelog.mjs index a8a855d..a740ab1 100644 --- a/scripts/release-changelog.mjs +++ b/scripts/release-changelog.mjs @@ -12,10 +12,12 @@ import { existsSync, readdirSync, readFileSync, + realpathSync, unlinkSync, writeFileSync, } from "node:fs"; import { join } from "node:path"; +import { fileURLToPath } from "node:url"; const CHANGELOG = "CHANGELOG.md"; const FRAGMENT_DIR = "changelog.d"; @@ -32,7 +34,7 @@ const HEADINGS = { // process end on its own with exitCode 1. process.exit() would race the // stderr write when stderr is a pipe (asynchronous on macOS), leaving the // caller a bare failure status with no reason attached. -class ReleaseError extends Error {} +export class ReleaseError extends Error {} const fail = (msg) => { throw new ReleaseError(msg); }; @@ -54,10 +56,10 @@ const isBlockConstruct = (l) => isRule(l.trim()) || isHeading(itemContent(l)); // a well-formed fragment file aborts the release, so an entry can never be // silently left out (e.g. a fragment created under a 'fix/' subdirectory // because the slug was taken verbatim from a branch name). -function collectFragments() { +function collectFragments(dir) { const fragments = []; - if (!existsSync(FRAGMENT_DIR)) return fragments; - const entries = readdirSync(FRAGMENT_DIR, { withFileTypes: true }).sort( + if (!existsSync(join(dir, FRAGMENT_DIR))) return fragments; + const entries = readdirSync(join(dir, FRAGMENT_DIR), { withFileTypes: true }).sort( (a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0), ); for (const entry of entries) { @@ -77,7 +79,7 @@ function collectFragments() { `'.<${CATEGORIES.join("|")}>.md' so the entry is not silently stranded.`, ); } - const body = readFileSync(join(FRAGMENT_DIR, name), "utf8").trim(); + const body = readFileSync(join(dir, FRAGMENT_DIR, name), "utf8").trim(); if (!body) fail(`${FRAGMENT_DIR}/${name}: empty fragment.`); const offending = body .split("\n") @@ -140,10 +142,10 @@ function mergeFragments(body, frags) { return chunks.join("\n\n"); } -function main() { - const { version } = JSON.parse(readFileSync("package.json", "utf8")); - const text = readFileSync(CHANGELOG, "utf8"); - const fragments = collectFragments(); +export function release(dir) { + const { version } = JSON.parse(readFileSync(join(dir, "package.json"), "utf8")); + const text = readFileSync(join(dir, CHANGELOG), "utf8"); + const fragments = collectFragments(dir); // Exactly one Unreleased heading. A second one silently strands entries: // only the first is ever cut, so anything filed under a later heading is @@ -185,22 +187,36 @@ function main() { // comes first) and would inject the version heading into the wrong place. const date = new Date().toISOString().slice(0, 10); writeFileSync( - CHANGELOG, + join(dir, CHANGELOG), text.slice(0, header.index) + `## Unreleased\n\n## ${version} — ${date}\n\n${merged}\n\n` + rest, ); - for (const f of fragments) unlinkSync(join(FRAGMENT_DIR, f.name)); - console.log( + for (const f of fragments) unlinkSync(join(dir, FRAGMENT_DIR, f.name)); + return ( `${CHANGELOG}: released '## ${version} — ${date}' from ${fragments.length} ` + - "fragment(s) plus the Unreleased section.", + "fragment(s) plus the Unreleased section." ); } -try { - main(); -} catch (e) { - if (!(e instanceof ReleaseError)) throw e; - console.error(`release-changelog: ${e.message}`); - process.exitCode = 1; +// import.meta.url names the script's real path, and argv[1] the path it was +// run by, which a symlink can differ from. When a module imports this one, +// argv[1] is that program's, and need not name a file at all. +function isMain() { + if (process.argv[1] === undefined) return false; + try { + return realpathSync(process.argv[1]) === fileURLToPath(import.meta.url); + } catch { + return false; + } +} + +if (isMain()) { + try { + console.log(release(process.cwd())); + } catch (e) { + if (!(e instanceof ReleaseError)) throw e; + console.error(`release-changelog: ${e.message}`); + process.exitCode = 1; + } } diff --git a/src/webhook.ts b/src/webhook.ts index eba3833..58d560f 100644 --- a/src/webhook.ts +++ b/src/webhook.ts @@ -78,11 +78,11 @@ export interface WebhookListenerOptions { } /** A bounded FIFO set: `add` returns true when the id was already present. */ -class DeliveryRing { +export class DeliveryRing { private readonly seen = new Set(); private readonly order: string[] = []; - constructor(private readonly capacity: number) {} + constructor(readonly capacity: number) {} add(id: string): boolean { if (this.seen.has(id)) return true; @@ -173,6 +173,10 @@ export class WebhookListener { return this.ring.size; } + get dedupCapacity(): number { + return this.ring.capacity; + } + // ── Request handling ── private handle(req: IncomingMessage, res: ServerResponse): void { diff --git a/test/integration.test.ts b/test/integration.test.ts index 0e342a9..1601559 100644 --- a/test/integration.test.ts +++ b/test/integration.test.ts @@ -121,6 +121,16 @@ async function refusal(call: Promise): Promise<{ code: number; message: /** JSON-RPC: the method does not exist on this connection. */ const ERR_METHOD_NOT_FOUND = -32601; + +// The server writes to the host in order on one stream, so once the answer to +// a later request is back, everything it sent before has arrived. +async function hostCaughtUp(h: Harness): Promise { + await h.host.sendRequest('tools/list', {}); +} + +function accepted(res: Response): void { + assert.ok(res.status >= 200 && res.status < 300, `the delivery was refused with ${res.status}`); +} /** MCPL §6.6: a capability the host did not grant. */ const ERR_CAPABILITY_DENIED = -32002; /** MCPL §8.7: the checkpoint is unknown or pruned. */ @@ -250,8 +260,8 @@ test('a host that grants nothing gets refusals on every tool, channel and push', // And no push: an event has nowhere to go. const deliver = await webhook(t, h); - await deliver('issues', fixture('issues.opened.json'), 'denied-1'); - await new Promise((r) => setTimeout(r, 50)); + accepted(await deliver('issues', fixture('issues.opened.json'), 'denied-1')); + await hostCaughtUp(h); assert.deepEqual(pushes(h), [], 'no push/event under an empty grant'); assert.deepEqual(incoming(h), [], 'and no channels/incoming'); @@ -315,8 +325,8 @@ test('a plain-MCP host sees tools only: no manifest, no push, no channels, no ro // And no event reaches the host: there is no push and no channel. const deliver = await webhook(t, h); - await deliver('issues', fixture('issues.opened.json'), 'plain-1'); - await new Promise((r) => setTimeout(r, 50)); + accepted(await deliver('issues', fixture('issues.opened.json'), 'plain-1')); + await hostCaughtUp(h); assert.deepEqual(h.hostSaw, [], 'the server sent the host nothing at all'); }); @@ -576,8 +586,8 @@ test('a disabled github.events stops the traffic at once, push and incoming alik h.host.sendNotification(method.FEATURE_SETS_UPDATE, { disabled: [EVENTS_FEATURE_SET] }); await until(() => !h.server.grant.isFeatureSetActive(EVENTS_FEATURE_SET), 'the notification to land'); - await deliver('issues', fixture('issues.closed.json'), 'after-1'); - await new Promise((r) => setTimeout(r, 50)); + accepted(await deliver('issues', fixture('issues.closed.json'), 'after-1')); + await hostCaughtUp(h); assert.equal(pushes(h).length, 1, 'no second push'); assert.equal(incoming(h).length, 1, 'and no second delivery'); }); diff --git a/test/release-changelog.test.ts b/test/release-changelog.test.ts index 9d4e42b..a39892e 100644 --- a/test/release-changelog.test.ts +++ b/test/release-changelog.test.ts @@ -1,7 +1,3 @@ -// Black-box coverage of scripts/release-changelog.mjs: each case runs the -// real script in a throwaway directory, so assembly, validation, and the -// deletion of consumed fragments are all exercised exactly as `npm version` -// would run them. import { test } from "node:test"; import assert from "node:assert/strict"; import { execFileSync } from "node:child_process"; @@ -10,11 +6,13 @@ import { mkdtempSync, readdirSync, readFileSync, + symlinkSync, writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; -import { fileURLToPath } from "node:url"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { release, ReleaseError } from "../scripts/release-changelog.mjs"; const SCRIPT = fileURLToPath( new URL("../scripts/release-changelog.mjs", import.meta.url), @@ -60,7 +58,16 @@ function setup(f: Fixture): string { function run(dir: string): { status: number; stdout: string; stderr: string } { try { - const stdout = execFileSync(process.execPath, [SCRIPT], { + return { status: 0, stdout: `${release(dir)}\n`, stderr: "" }; + } catch (e) { + if (!(e instanceof ReleaseError)) throw e; + return { status: 1, stdout: "", stderr: `release-changelog: ${e.message}\n` }; + } +} + +function runCli(dir: string, script = SCRIPT): { status: number; stdout: string; stderr: string } { + try { + const stdout = execFileSync(process.execPath, [script], { cwd: dir, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], @@ -200,3 +207,39 @@ for (const [name, fixture, message] of refusals) { assert.deepEqual(pending(dir), before.pending, "no fragment may be deleted on refusal"); }); } + +test("the CLI releases from the directory it runs in, also when run through a symlink", () => { + const dir = setup({ fragments: { "a.fixed.md": "- Fixed thing.\n" } }); + const linked = join(dir, "release-changelog.mjs"); + symlinkSync(SCRIPT, linked); + const r = runCli(dir, linked); + assert.equal(r.status, 0, r.stderr); + assert.match(r.stdout, /^CHANGELOG\.md: released '## 1\.1\.0 — \d{4}-\d{2}-\d{2}' from 1 fragment\(s\) plus the Unreleased section\.\n$/); + assert.equal(section(changelog(dir), "1.1.0"), "### Fixed\n\n- Fixed thing."); + assert.deepEqual(pending(dir), ["README.md"]); +}); + +test("the CLI refuses with exit status 1 and its reason on stderr, touching nothing", () => { + const dir = setup({ version: "1.0.0", fragments: { "a.fixed.md": "- x\n" } }); + const before = { changelog: changelog(dir), pending: pending(dir) }; + const r = runCli(dir); + assert.equal(r.status, 1, `expected refusal, got exit ${r.status}:\n${r.stdout}${r.stderr}`); + assert.equal(r.stdout, ""); + assert.match(r.stderr, /^release-changelog: .*'## 1\.0\.0' section already exists/); + assert.equal(changelog(dir), before.changelog, "CHANGELOG.md must be untouched"); + assert.deepEqual(pending(dir), before.pending, "no fragment may be deleted on refusal"); +}); + +test("importing the script runs nothing, whatever argv[1] names", () => { + const dir = setup({ fragments: { "a.fixed.md": "- x\n" } }); + const before = { changelog: changelog(dir), pending: pending(dir) }; + const code = `process.argv[1] = "test/*.test.ts"; await import(${JSON.stringify(pathToFileURL(SCRIPT).href)});`; + const stdout = execFileSync(process.execPath, ["--input-type=module", "-e", code], { + cwd: dir, + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }); + assert.equal(stdout, ""); + assert.equal(changelog(dir), before.changelog); + assert.deepEqual(pending(dir), before.pending); +}); diff --git a/test/webhook.test.ts b/test/webhook.test.ts index 1aea9c2..dc3affb 100644 --- a/test/webhook.test.ts +++ b/test/webhook.test.ts @@ -1,8 +1,8 @@ /** * Webhook listener (TASK-SRHKG.1.3): the order of the checks is the test. * - * Every case below goes through a real `node:http` server on a loopback port - * and a real `fetch`, because the properties under test — "rejected before + * Every case below but the dedup ring's goes through a real `node:http` server + * on a loopback port and a real `fetch`, because the properties under test — "rejected before * parsing", "2xx returned before the event is processed", "one stderr line" — * are properties of the request path and not of a function. */ @@ -13,7 +13,7 @@ import { readFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { test, type TestContext } from 'node:test'; -import { MAX_BODY_BYTES, WebhookListener, webhookListenerFromConfig } from '../src/webhook.js'; +import { DELIVERY_DEDUP_RING, DeliveryRing, MAX_BODY_BYTES, WebhookListener, webhookListenerFromConfig } from '../src/webhook.js'; import { CheckGate } from '../src/normalize.js'; import { Poller, type IngressState } from '../src/poller.js'; import type { ChecksSummary, Config, Logger, NormalizedEvent, RepoRef } from '../src/types.js'; @@ -208,25 +208,20 @@ test('a replayed delivery id is dropped and the event is emitted once', async (t await r.listener.whenIdle(); assert.equal(r.events.length, 1); assert.equal(r.log.lines.filter((l) => l.includes('replay')).length, 1, r.log.text()); + assert.equal(r.listener.rememberedDeliveries, 1); + assert.equal(r.listener.dedupCapacity, 2000); }); -test('the dedup gate remembers 2000 ids and then forgets the oldest', async (t) => { - const r = await rig(t); - const body = fixture('ping.json'); - // `ping` is answered after the gate check, so it exercises dedup cheaply. - for (let i = 0; i < 2000; i++) await r.deliver('ping', body, `gate-${i}`); - assert.equal(r.listener.rememberedDeliveries, 2000); - // Still remembered at the capacity boundary. - assert.equal((await r.deliver('ping', body, 'gate-0')).status, 200); - assert.equal(r.log.lines.filter((l) => l.includes('replay')).length, 1); - - // One more distinct id than the gate holds evicts `gate-1` (the oldest - // after `gate-0` was re-seen at the tail is `gate-1`). - for (let i = 2000; i < 2002; i++) await r.deliver('ping', body, `gate-${i}`); - assert.equal(r.listener.rememberedDeliveries, 2000); - assert.equal((await r.deliver('ping', body, 'gate-1')).status, 200); - // Re-accepted, not reported as a replay: exactly one replay line so far. - assert.equal(r.log.lines.filter((l) => l.includes('replay')).length, 1, r.log.text()); +test('the dedup ring remembers 2000 ids and then forgets the oldest', () => { + const ring = new DeliveryRing(DELIVERY_DEDUP_RING); + for (let i = 0; i < 2000; i++) assert.equal(ring.add(`gate-${i}`), false, `gate-${i} is new`); + assert.equal(ring.size, 2000); + assert.equal(ring.add('gate-0'), true, 'still remembered at the capacity boundary'); + + for (let i = 2000; i < 2002; i++) assert.equal(ring.add(`gate-${i}`), false, `gate-${i} is new`); + assert.equal(ring.size, 2000); + assert.equal(ring.add('gate-0'), false, 'two more ids than the ring holds have evicted gate-0, replayed or not'); + assert.equal(ring.add('gate-1'), false, 'and gate-1'); }); // ─────────────────────────────────────────────────────────────────────────────