diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 876f980b..586fd51f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,10 +1,13 @@ name: CI on: + # The per-feature PRs are a stack whose bases are other `ds/*` branches; with the + # trigger list limited to the integration branches, none of them could ever run CI. push: - branches: [dot-skill-test, dot-skill, main] + branches: [dot-skill-test, dot-skill, main, 'ds/**'] pull_request: - branches: [dot-skill-test, dot-skill, main] + branches: [dot-skill-test, dot-skill, main, 'ds/**'] + workflow_dispatch: jobs: test: diff --git a/docs/evidence/pr-20-repo-hygiene.md b/docs/evidence/pr-20-repo-hygiene.md new file mode 100644 index 00000000..6b0a6f3b --- /dev/null +++ b/docs/evidence/pr-20-repo-hygiene.md @@ -0,0 +1,115 @@ +# PR-20 · 仓库忽略规则:把用户数据、凭据与本地证据的边界补回来 + +- 分支:`ds/20-repo-hygiene`(worktree `/tmp/dot-skill-test-20`,已本地合并进 `dot-skill-test`) +- 依赖:无 +- 发现方式:第 26 轮对抗式验证(独立子代理)复核「截图不入库 / 无 Python 残留」时, + 顺手 `git check-ignore` 了真实使用会产生的路径,发现 `.gitignore` 只剩 10 行 +- 本文纯文字,无截图;证据目录 `dst-evidence/` 不入库 + +## 1. 缺陷 + +`.gitignore` 在 v2 期间被一次**合并**整段替换。提交历史里它的行数: + +| 提交 | 行数 | 说明 | +| --- | --- | --- | +| `6a0b31a` | 27 | AgentSkills 目录结构重构 | +| `653a5a6` | 34 | dot-skill 引擎文件回归 | +| `05ff594` | 40 | 冻结 v2 契约,追加 `dst-evidence/`、`*.evidence.local` | +| `07d1ba5` | **43** | 合成账本夹具,追加 fixtures 的 `knowledge/` 例外 | +| (某次合并) | **5** | 只剩 knowledge 相关规则 | +| `06cea58` | 10 | 追加 `!src/knowledge/**`(本意是 +5 行的补充提交) | +| `238f9b8`(HEAD) | 10 | 一直是 10 行 | + +`06cea58` 的提交信息写的是"例外必须写在排除规则之后",但它基于的父提交已经只剩 5 行, +所以它**补不回**丢掉的东西。丢失的规则: + +- `skills/*` + 三个 example 白名单 —— **真实人物的档案**(文件头第一行就是 + `# Generated Distilly outputs (user data, not committed)`) +- `.distilly/`、`.colleague-skill/` —— **凭据配置** +- `playwright-data/` —— 本地浏览器 profile +- `.DS_Store`、`Thumbs.db` —— OS 文件 +- `dst-evidence/`、`*.evidence.local` —— 契约 §4 要求的"截图不入库"机制 + +### 为什么一直没被发现 + +三条门禁都只看**已经跟踪的东西**,而这次丢的是"以后会被创建的东西": + +- `git status` 干净 —— 因为没人真的在仓库里跑过 `harvest` +- `audit-objective.mjs` 第 10 条只查 `git ls-files` 里有没有证据图片 → 0 张,通过 +- `check_release.mjs` 不查忽略规则 + +## 2. 改动前 → 改动后(可复算) + +复现:在仓库里造出真实使用会产生的文件,然后看 `git status`。 + +```bash +mkdir -p skills/colleague/real_person/knowledge/text skills/colleague/real_person/views \ + .distilly playwright-data dst-evidence/screenshots +echo '{"name":"真人"}' > skills/colleague/real_person/meta.json +echo '{"api_key":"sk-live-DEADBEEF"}' > .distilly/config.json +echo 'cookie' > playwright-data/state.json +touch .DS_Store +echo png > dst-evidence/screenshots/real-person-profile.png +git status --porcelain +``` + +| | before(`238f9b8`) | after(本分支) | +| --- | --- | --- | +| `git status --porcelain` | `?? .DS_Store`、`?? .distilly/`、`?? dst-evidence/`、`?? playwright-data/`、`?? skills/colleague/real_person/` | 空(只剩 `.gitignore` 自己的修改) | +| `git add -A` 会收进 | `.DS_Store`、`.distilly/config.json`(**明文 API key**)、`dst-evidence/screenshots/real-person-profile.png`、`playwright-data/state.json`、`skills/colleague/real_person/meta.json` | 无 | +| 反向误伤 | `src/knowledge/**`、`skills/colleague/example_*`、夹具 `knowledge/` 均可见 ✅ | 同样全部可见 ✅ | + +`origin/dot-skill`(默认分支)上这些路径**全部**被忽略,`git status` 为空 —— 所以这是 +v2 分支引入的**回归**,不是"本来就没管"。 + +逐条命中的规则(after): + +``` +skills/colleague/real_person/meta.json .gitignore:4:skills/colleague/* +skills/colleague/real_person/knowledge/text/a.md .gitignore:4:skills/colleague/* +.distilly/config.json .gitignore:21:.distilly/ +playwright-data/state.json .gitignore:26:playwright-data/ +.DS_Store .gitignore:47:.DS_Store +dst-evidence/screenshots/real-person-profile.png .gitignore:51:dst-evidence/ +knowledge/raw/a.txt .gitignore:38:knowledge/ +``` + +## 3. 门禁加强(否则同类事情还会溜过) + +只修文件不够:原来那条审计看得见"现在的仓库",看不见"以后的仓库"。 + +### `tests/gitignore.test.mjs`(新增,3 条) + +用 `git check-ignore -q`(按模式判定,路径无需存在,因此测试不改工作树)从两侧钉死边界: + +- 必须被忽略:真实人物目录(`meta.json` / `persona.md` / `knowledge/raw` / `knowledge/text` / + `evidence/derived` / `evidence/renders` / `views/*.html`)、根级 `knowledge/raw`、 + `.distilly/`、`.colleague-skill/`、`playwright-data/`、`dst-evidence/`、`.DS_Store`、`Thumbs.db` +- 必须仍然可见:`src/knowledge/**`(含"以后新增的模块")、夹具 `knowledge/`、三个 example +- 顺序:`!src/knowledge/` 与 `!src/derive/fixtures/**/knowledge/` 必须出现在最后一条 + `knowledge/` 规则**之后**(git 最后匹配者胜,顺序错了就静默失效) + +在缺陷版 `.gitignore` 上实测:**2 条失败**(`# pass 1 / # fail 2`);修复版上 3/3 通过。 + +### `scripts/audit-objective.mjs` 第 10 条 + +原来只查 `git ls-files` 里的证据图片数。现在同一条里追加机制检查:对 9 个"真实使用会 +创建"的路径逐个 `git check-ignore`,必须全部命中。缺陷版实测 **16/17**,修复版 **17/17** +(证据串新增 `ignore rules cover 9/9 created-in-tree paths`)。 + +## 4. 测试结果 + +``` +$ node --test tests/gitignore.test.mjs # 3 pass / 0 fail(修复版) +$ node --test tests/gitignore.test.mjs # 1 pass / 2 fail(缺陷版,证明这条测试咬得住) +$ node --test tests/*.test.mjs # 381 pass / 0 fail(39 → 40 个文件) +$ node scripts/audit-objective.mjs --skip-acceptance # 17/17(缺陷版 16/17) +``` + +## 5. 已知缺口 + +- `node --test`(不带参数,即 CI 与 `npm test` 实际跑的命令)在本轮**是失败的**,原因是 + `scripts/blind-test.mjs` 被 Node 的默认发现规则(`**/*-test.mjs`)当成测试文件收集, + 裸跑打印用法并 exit 2。这是**独立缺陷**,不在本 PR 范围内,另开 PR 处理。 +- 本 PR 只恢复忽略规则,不改任何运行时代码;`skills/colleague/example_*` 等已跟踪文件 + 不受影响(已跟踪文件本来就不受 `.gitignore` 约束)。 diff --git a/package.json b/package.json index 81fefabe..fc2c7a26 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "CITATION.cff" ], "scripts": { - "test": "node --test \"tests/*.test.mjs\"", + "test": "node --test tests/*.test.mjs", "prepack": "node bin/distilly.mjs --check-package" }, "keywords": [ diff --git a/scripts/acceptance.mjs b/scripts/acceptance.mjs index 84ef7b08..fddeeaf8 100644 --- a/scripts/acceptance.mjs +++ b/scripts/acceptance.mjs @@ -183,10 +183,17 @@ try { // 4. visual-check(八项) const vcScript = path.join(root, 'scripts/visual-check.mjs'); const vc = spawnSync('node', [vcScript, htmlPath, '--out', evidenceDir], { cwd: workdir, encoding: 'utf8' }); + // The hint that playwright is missing goes to **stderr**, so a row that quoted + // only stdout went red with an empty reason — a gate nobody can act on. + const vcDetail = + (vc.stdout ?? '').trim().split('\n').slice(-3).join(' / ') || + (vc.stderr ?? '').trim().split('\n').slice(-2).join(' / '); if (/Cannot find module|ENOENT/.test(vc.stderr ?? '')) { record('visual-check 可用', false, 'scripts/visual-check.mjs 尚不存在(ds/03-render 的产出)'); + } else if (/DISTILLY_PLAYWRIGHT_ROOT/.test(vc.stderr ?? '')) { + record('visual-check 可用', false, '未提供 playwright:设 DISTILLY_PLAYWRIGHT_ROOT=<含 node_modules 的目录>'); } else { - record('visual-check 八项通过', vc.status === 0, (vc.stdout ?? '').trim().split('\n').slice(-3).join(' / ')); + record('visual-check 八项通过', vc.status === 0, vcDetail); } } catch (error) { record('验收流程未中断', false, String(error.message).split('\n')[0]); diff --git a/scripts/audit-objective.mjs b/scripts/audit-objective.mjs index 5c186a7e..4baa0933 100644 --- a/scripts/audit-objective.mjs +++ b/scripts/audit-objective.mjs @@ -33,6 +33,22 @@ const record = (demand, ok, evidence, { gap = false } = {}) => { }; const git = (...args) => execFileSync("git", args, { cwd: root, encoding: "utf8" }).trim(); +/** + * `git rev-parse --verify --quiet ` → the ref's sha, or `null` when the ref + * does not exist. + * + * `git()` cannot be used for an existence probe: it throws on a non-zero exit, and + * a missing ref exits 1 with empty output. That turned the push row — the row whose + * whole job is to report "this branch is on no remote" — into a crash of the entire + * audit inside CI, where the checkout has no `origin/dot-skill-test` tracking ref. + */ +const gitRef = (ref) => { + try { + return execFileSync("git", ["rev-parse", "--verify", "--quiet", ref], { cwd: root, encoding: "utf8" }).trim(); + } catch { + return null; + } +}; /** 1. Node single stack: no Python left anywhere in the tree. */ { @@ -219,10 +235,16 @@ const git = (...args) => execFileSync("git", args, { cwd: root, encoding: "utf8" if (skipAcceptance) { record("端到端验收(本审计已跳过)", true, "--skip-acceptance was passed", { gap: true }); } else { + // `DISTILLY_PLAYWRIGHT_ROOT` is **passed through, never invented**. It used to + // default to `/tmp/audit-mcp`, which made this audit's result depend on a + // directory in a volatile temp path: on any other machine — or on this one after + // a reboot — the end-to-end row went red for a reason nothing stated. A caller + // that has playwright sets the variable (see docs/evidence/pr-03-render.md); a + // caller that does not gets a named, actionable failure from acceptance itself. const output = execFileSync(process.execPath, [join(root, "scripts", "acceptance.mjs")], { cwd: root, encoding: "utf8", - env: { ...process.env, DISTILLY_PLAYWRIGHT_ROOT: process.env.DISTILLY_PLAYWRIGHT_ROOT ?? "/tmp/audit-mcp" }, + env: { ...process.env }, }); const summary = output.trim().split("\n").pop() ?? ""; const match = /(\d+)\/(\d+)\s*通过/.exec(summary); @@ -236,22 +258,31 @@ if (skipAcceptance) { // `upstream`. Resolve whichever remote actually has the branch, and say so when // none does, instead of throwing on a hardcoded name. const remotes = git("remote").split("\n").map((line) => line.trim()).filter(Boolean); - const tracking = remotes - .map((remote) => `${remote}/dot-skill-test`) - .find((ref) => git("rev-parse", "--verify", "--quiet", ref) !== ""); + const tracking = remotes.map((remote) => `${remote}/dot-skill-test`).find((ref) => gitRef(ref) !== null); const ahead = tracking === undefined ? null : Number(git("rev-list", "--count", `${tracking}..HEAD`)); - const dirty = git("status", "--porcelain"); + const dirty = git("status", "--porcelain").split("\n").filter(Boolean); // "Pushed" is the thing the user asked for (an off-site copy). A PR is a // separate, still-unrequested step, so it is reported rather than required. - const pushed = ahead === 0; + // A dirty tree is counted as **the same risk** as an unpushed commit — the title + // has always claimed it, and uncommitted work is the more volatile of the two — + // so the check now enforces what the title says instead of merely printing it. + const pushed = ahead === 0 && dirty.length === 0; + + // In CI the demand cannot apply: the checkout *came from* the remote, and there + // is no tracking ref for the branch to measure against. Recording that as a gap + // keeps it visible; pretending to have verified it would be the silent pass this + // audit exists to prevent. + const inCi = process.env.GITHUB_ACTIONS === "true"; record( "推送:集成分支已推送到远端(异地备份),工作树干净", - pushed, - tracking === undefined - ? "no remote carries dot-skill-test; every commit exists only on this machine" - : `${tracking}: ${ahead} commit(s) ahead; working tree ${dirty === "" ? "clean" : "dirty"}; PR bodies staged in dst-evidence/PR-BODIES/`, - { gap: !pushed }, + inCi ? true : pushed, + inCi + ? `CI checkout: ${remotes.length} remote(s) configured, no local record of the branch, so "off-site" is this run's own source — not verified here` + : tracking === undefined + ? "no remote carries dot-skill-test; every commit exists only on this machine" + : `${tracking}: ${ahead} commit(s) ahead; working tree ${dirty.length === 0 ? "clean" : `${dirty.length} path(s) dirty`}; PR bodies staged in dst-evidence/PR-BODIES/`, + { gap: inCi || !pushed }, ); } diff --git a/src/collect/slack.mjs b/src/collect/slack.mjs index 51515dfd..9dd35cb8 100644 --- a/src/collect/slack.mjs +++ b/src/collect/slack.mjs @@ -88,9 +88,15 @@ export function distillyHome(env = process.env) { } export function credentialPaths(env = process.env) { + // `env.HOME ?? homedir()`, matching `kit.mjs`: the pre-rename + // `~/.colleague-skill/` fallback has to move when a caller points `HOME` + // somewhere else. Reading the real home here ignored `env.HOME` — so an isolated + // run (a test, a sandboxed collect) could still pick up the credential sitting in + // the developer's own home, and the two channels disagreed about which file they + // had just read. return { primary: join(distillyHome(env), CONFIG_FILE), - legacy: join(homedir(), LEGACY_CONFIG_FILE), + legacy: join(env?.HOME ?? homedir(), LEGACY_CONFIG_FILE), }; } diff --git a/tests/collect.test.mjs b/tests/collect.test.mjs index 1974b3cb..7413ab61 100644 --- a/tests/collect.test.mjs +++ b/tests/collect.test.mjs @@ -86,6 +86,26 @@ function sandbox(name = "case") { }; } +/** + * Every file written under `dir`, relative and sorted — for failure messages. + * + * A failing "the page was not written" assertion used to say only `false !== true`, + * which is unactionable from a CI log: it cannot distinguish "nothing was written" + * from "something was written somewhere else". + */ +function written(dir) { + const out = []; + const walk = (current, prefix) => { + for (const entry of readdirSync(current, { withFileTypes: true })) { + const rel = prefix ? `${prefix}/${entry.name}` : entry.name; + if (entry.isDirectory()) walk(join(current, entry.name), rel); + else out.push(rel); + } + }; + if (existsSync(dir)) walk(dir, ""); + return out.length === 0 ? "(nothing)" : out.sort().join(", "); +} + function json(body, { status = 200, headers = {} } = {}) { return new Response(JSON.stringify(body), { status, headers }); } @@ -548,10 +568,21 @@ test("CLI success path: a fake key never reaches stdout, stderr or the receipt", assert.ok(!run.stdout.includes(SECRET.slack), "stdout leaked the credential"); assert.ok(!run.stderr.includes(SECRET.slack), "stderr leaked the credential"); const receipt = receiptOf(run.stdout); - assert.equal(receipt.ok, true); + // The messages name what was seen: a bare `false !== true` on either of these made + // a CI-only failure impossible to diagnose from the log alone. + assert.equal(receipt.ok, true, `receipt was not ok: ${JSON.stringify(receipt)}`); assert.equal(receipt.credential_file, "slack_config.json"); assert.ok(!JSON.stringify(receipt).includes(SECRET.slack)); - assert.equal(box.exists("knowledge/raw/slack/C0123-p001.json"), true); + assert.equal( + box.exists("knowledge/raw/slack/c0123-p001.json"), + true, + // The channel id is `C0123` and the **file name is slugged to `c0123`** + // (`writeRaw` runs the name through `slug()`), while the receipt keeps + // `channel_id: "C0123"`. This assertion used to read `C0123-p001.json` and + // passed on macOS only because APFS compares names case-insensitively; on the + // Linux runner it failed while the file sat right there in the listing. + `the page was not written; receipt=${JSON.stringify(receipt)} stderr=${run.stderr} tree=${written(box.work)}`, + ); }); test("CLI failure path: a rejected key leaks nothing and names where to reconfigure", () => { diff --git a/tests/feishu-mcp.test.mjs b/tests/feishu-mcp.test.mjs index eff6c07e..ea4f54db 100644 --- a/tests/feishu-mcp.test.mjs +++ b/tests/feishu-mcp.test.mjs @@ -36,7 +36,22 @@ function sandbox() { const root = mkdtempSync(join(tmpdir(), "dst-mcp-")); const home = join(root, "distilly"); mkdirSync(home, { recursive: true }); - return { root, home, work: join(root, "work"), config: { app_id: "cli_x", app_secret: SECRET } }; + const config = { app_id: "cli_x", app_secret: SECRET }; + // An **isolated home with the credential written into it**, plus the env that + // points at it. Without this the CLI-routing tests below read the developer's + // real home: `credentialPaths` falls back to `~/.colleague-skill/`, this + // machine still has that pre-rename directory, and the run found a credential + // there. Locally green, and red in CI, where the file does not exist — the exact + // "a test that depends on whose laptop it runs on" failure this file already + // warned about in its missing-credential test. + writeFileSync(join(home, "feishu_config.json"), JSON.stringify(config, null, 2)); + return { + root, + home, + work: join(root, "work"), + config, + env: { DISTILLY_HOME: home, HOME: home }, + }; } /** An MCP transport that answers from a script and records every call. */ @@ -200,13 +215,17 @@ test("cli: --mode mcp routes to the MCP client and needs a target", async () => const box = sandbox(); const transport = fakeTransport({ result: [{ type: "text", text: JSON.stringify({ items: textMessages(3) }) }] }); - const result = await runCollectCli(["--mode", "mcp", "--chat-id", "oc_demo", "--person", "demo", "--base-dir", box.work, "--json"], { transport }); + const result = await runCollectCli(["--mode", "mcp", "--chat-id", "oc_demo", "--person", "demo", "--base-dir", box.work, "--json"], { + transport, + env: box.env, + }); assert.equal(result.ok, true, JSON.stringify(result.receipt)); assert.equal(transport.calls.length, 1); assert.deepEqual(transport.calls[0].args, { chat_id: "oc_demo", page_size: 50 }); const lines = []; const human = await runCollectCli(["--mode", "mcp", "--chat-id", "oc_demo", "--person", "demo", "--base-dir", box.work], { transport, + env: box.env, stdout: (line) => lines.push(line), stderr: () => {}, }); diff --git a/tests/gitignore.test.mjs b/tests/gitignore.test.mjs new file mode 100644 index 00000000..f3fb8d73 --- /dev/null +++ b/tests/gitignore.test.mjs @@ -0,0 +1,113 @@ +/** + * The repository's ignore rules are a privacy boundary, not a convenience. + * + * `skills///` holds a real person's distilled profile and + * `.distilly/` holds API keys. Both are produced in the working tree, so the only + * thing keeping them out of a commit is `.gitignore`. A merge once replaced that + * file with a ten-line version that kept only the `knowledge/` rules, and the loss + * was invisible: nothing was tracked that should not be, `git status` stayed clean, + * and the objective audit only looked at what was *already* tracked. + * + * These assertions pin the boundary from both sides — what must be ignored, and + * what must stay visible — so a future merge cannot quietly drop a rule again. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import path from "node:path"; + +const root = path.dirname(path.dirname(fileURLToPath(import.meta.url))); + +/** `git check-ignore` answers by pattern alone, so the paths need not exist. */ +const ignored = (candidate) => + spawnSync("git", ["check-ignore", "-q", "--", candidate], { cwd: root }).status === 0; + +const inGitWorkTree = () => + spawnSync("git", ["rev-parse", "--is-inside-work-tree"], { cwd: root, encoding: "utf8" }).stdout?.trim() === + "true"; + +const skip = inGitWorkTree() ? false : "not a git work tree"; + +/** + * Paths a person's data, a key, or a local artefact would land on. Every one of + * these must be ignored, because every one of them is created by ordinary use. + */ +const MUST_BE_IGNORED = [ + // A real person's directory — the profile, the spine, the rendered pages. + "skills/colleague/some_real_person/meta.json", + "skills/colleague/some_real_person/persona.md", + "skills/colleague/some_real_person/knowledge/raw/export.json", + "skills/colleague/some_real_person/knowledge/text/export.md", + "skills/colleague/some_real_person/evidence/derived/stats.json", + "skills/colleague/some_real_person/evidence/renders/view-print.png", + "skills/colleague/some_real_person/views/some_real_person.html", + // A root-level export, for the same reason. + "knowledge/raw/export.json", + // Credentials: current path and the legacy one. + ".distilly/config.json", + ".distilly/consent.json", + ".colleague-skill/config.json", + // Local browser profile written by visual-check. + "playwright-data/state.json", + // Evidence stays out of the repository (CONTRACT §4). + "dst-evidence/SCREENSHOTS.md", + "dst-evidence/screenshots/pr-20/view-print.png", + // OS noise. + ".DS_Store", + "Thumbs.db", +]; + +/** + * Paths that must stay visible. These are the re-includes: get the ordering wrong + * and a newly added module or fixture is silently dropped from `git add`. + */ +const MUST_STAY_VISIBLE = [ + "src/knowledge/store.mjs", + "src/knowledge/anchors.mjs", + "src/knowledge/ledger.mjs", + "src/knowledge/identity.mjs", + "src/knowledge/a_module_added_later.mjs", + "src/derive/fixtures/synthetic-group/knowledge/index.json", + "src/derive/fixtures/synthetic-group/knowledge/text/group-chat.md", + // The bundled examples must remain tracked; the ignore rule only covers the rest. + "skills/colleague/example_zhangsan/meta.json", + "skills/colleague/example_zhangsan/persona.md", + "skills/colleague/example_tianyi/meta.json", + "skills/colleague/example_jiaxiu/meta.json", +]; + +test("user data, credentials and local evidence are all ignored", { skip }, () => { + const leaked = MUST_BE_IGNORED.filter((candidate) => !ignored(candidate)); + assert.deepEqual(leaked, [], `these paths are not ignored and would be committed by \`git add -A\`: ${leaked.join(", ")}`); +}); + +test("source modules and bundled examples stay visible", { skip }, () => { + const hidden = MUST_STAY_VISIBLE.filter(ignored); + assert.deepEqual( + hidden, + [], + `these paths are ignored, so a newly added file would be lost from \`git add\` without a word: ${hidden.join(", ")}`, + ); +}); + +test("the re-includes come after the rule they override", { skip }, () => { + // Git applies `.gitignore` top to bottom and the last match wins, so an + // unanchored `knowledge/` placed after a `!src/knowledge/**` re-excludes it. + // (`readFileSync` comes from the top-level import: this file is ESM, so a + // stray `require` here is a reconstruction artefact, not a style choice.) + const lines = readFileSync(path.join(root, ".gitignore"), "utf8") + .split("\n") + .map((line) => line.trim()) + .filter((line) => line !== "" && !line.startsWith("#")); + const lastKnowledge = lines.lastIndexOf("knowledge/"); + assert.notEqual(lastKnowledge, -1, "the `knowledge/` rule itself must exist"); + for (const reinclusion of ["!src/knowledge/", "!src/derive/fixtures/**/knowledge/"]) { + assert.ok( + lines.indexOf(reinclusion) > lastKnowledge, + `${reinclusion} appears before the last \`knowledge/\` rule, so the later rule re-excludes it`, + ); + } +}); diff --git a/tests/package-payload.test.mjs b/tests/package-payload.test.mjs index e42a7e31..64bfbf5f 100644 --- a/tests/package-payload.test.mjs +++ b/tests/package-payload.test.mjs @@ -17,7 +17,7 @@ import test from "node:test"; import assert from "node:assert/strict"; -import { mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { fileURLToPath } from "node:url"; import path from "node:path"; @@ -27,6 +27,25 @@ import { payloadEntries, validatePayload } from "../bin/distilly.mjs"; const root = path.dirname(path.dirname(fileURLToPath(import.meta.url))); const manifest = JSON.parse(readFileSync(path.join(root, "package.json"), "utf8")); +/** + * The files one `npm test` argument names, resolving a single `*` against the tree. + * + * Deliberately not a general globber: what the command must look like is asserted + * separately, and a hand-rolled full glob would only be a second thing to keep + * correct. + */ +function namedFiles(arg) { + if (!path.basename(arg).includes("*")) return existsSync(path.join(root, arg)) ? [arg] : []; + const pattern = new RegExp( + `^${path + .basename(arg) + .split("*") + .map((part) => part.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")) + .join(".*")}$`, + ); + return readdirSync(path.join(root, path.dirname(arg))).filter((name) => pattern.test(name)); +} + /** * A root that really has every `payloadEntries` path (symlinked, so the check * sees them) but the candidate `files` list under test. @@ -114,18 +133,36 @@ test("these fixtures never write through to the repository manifest", () => { assert.equal(readFileSync(manifestPath, "utf8"), before, "the repository manifest must be untouched"); }); -test("`npm test` and CI run the same command, and it names the tests directory", () => { +test("`npm test` and CI run the same command, and every path it names exists", () => { // A bare `node --test` also collects `scripts/blind-test.mjs` (`**/*-test.mjs`) - // and records its usage error as a failing test — which is how CI went red - // while every local command looked green. + // and records it as a passing "test" — which is how CI went red while every local + // command looked green. + // + // This used to assert the command's exact text, `node --test "tests/*.test.mjs"`. + // That froze a command which **does not run on the oldest Node the package claims + // to support**: the quotes stop the shell expanding the glob, and Node 20's + // `--test` takes literal paths only, so `npm test` died with `Could not find + // '…/tests/*.test.mjs'` on the Node 20 leg of the matrix while passing on Node 22. + // + // The unquoted glob is the only form both legs accept: the shell expands it, so + // Node 20 receives real paths (as it requires), while Node 22 — which treats a + // positional argument as a glob of its own and refuses a bare directory with + // `Cannot find module '…/tests'` — receives paths it can also handle. const ci = readFileSync(path.join(root, ".github", "workflows", "ci.yml"), "utf8"); assert.match(ci, /^\s*run: npm test\s*$/m, "CI must invoke `npm test`"); - assert.equal(manifest.scripts.test, 'node --test "tests/*.test.mjs"'); - assert.equal( - /\bnode --test\s*$/.test(manifest.scripts.test), - false, - "`npm test` must not be a bare `node --test`: it would collect scripts/ as tests", - ); + + const argv = manifest.scripts.test.split(/\s+/); + assert.equal(argv[0], "node"); + assert.equal(argv[1], "--test"); + assert.ok(argv.length > 2, "`npm test` must not be a bare `node --test`: it would collect scripts/ as tests"); + for (const arg of argv.slice(2)) { + assert.equal( + /["']/.test(arg), + false, + `no quoting in \`npm test\`: a quoted glob reaches Node 20 unexpanded, and Node 20 has no glob support (got ${arg})`, + ); + assert.ok(namedFiles(arg).length > 0, `${arg} must name at least one file that exists`); + } }); test("`engines` matches what CI actually tests", () => {