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-16-discord-notion.md b/docs/evidence/pr-16-discord-notion.md new file mode 100644 index 00000000..8e9c6737 --- /dev/null +++ b/docs/evidence/pr-16-discord-notion.md @@ -0,0 +1,55 @@ +# PR-16 · Discord 与 Notion 两个采集渠道 + +- 分支:`ds/16-discord-notion`(本地,已合并进 `dot-skill-test`) +- 交付:`src/collect/kit.mjs`、`src/collect/discord.mjs`、`src/collect/notion.mjs`、 + `src/parse/chat.mjs`(透传 method / 凭据来源 / 认 `author.username`)、 + `src/commands/credentialed.mjs`、`tests/collect-discord-notion.test.mjs` + +## 1. 为什么 + +`CONTRACT.md` §1 的采集命令行写着 8 个渠道,此前只实现 4 个(飞书/Slack/钉钉/X)。 +未实现的两个渠道此前回 `collect/planned-channel` 并写明所需凭据——诚实,但仍是缺口。 +这一轮把 Discord(bot token)与 Notion(内部集成 token)补上,`PENDING_CHANNELS` +从 4 条缩到 2 条(reddit / gmail)。 + +## 2. 三条纪律,与已有渠道一致 + +| 纪律 | 实现 | +| --- | --- | +| 只读 | Discord 读 API 全是 GET,白名单就只列 GET;Notion 的读路径几乎全是 POST(search / 数据库查询),白名单**显式**列出这两个 POST 并拒绝其它一切。越权动词在建立连接之前被拒 | +| 凭据不进回执 | 只允许出现配置文件名(`discord_config.json` / `notion_config.json`);`redact`/`scrub` 覆盖回执的每个字段(含嵌套数组) | +| 逐字入库 + 可派生 | 原始页 verbatim 落 `knowledge/raw//`,同一次采集把它变成带锚点正文(Discord 交给**已有的导出解析器**,Notion 走段落化),一页/一页一个账本条目 | + +分页与限流:Discord 用 `before` 游标(一页不足 page size 即结束),429 读 body 里的 +`retry_after`(**秒**)退避——测试断言睡的是 1500ms;Notion 用 `next_cursor`。 +两处都支持断点续采(游标写在 `$DISTILLY_HOME/state/`)。 + +## 3. 顺带修好的两处解析器透传 + +- `parseChat` 现在尊重调用方的 `method` 与 `credentialed/credential_source/credential_file`: + 导出来的是 `user-export`,API 采集来的是 `api-bot-token`——否则采集来的页面会在账本里 + 冒充用户导出。 +- 说话人取名认 REST 的 `author.username`(导出用 `name`,API 用 `username`): + 同一批消息无论是下载的还是采集的,人名一致(`林工` 而不是 `u1`)。 +- 顺带的一致性变化:chat 的正文文件名现在随 harvest 的 source 标签(默认取输入目录名), + 与其它格式一致;`--source chat` 可恢复旧名。 + +## 4. 怎么验 + +```bash +node --test tests/collect-discord-notion.test.mjs # 10 条 +node --test tests/*.test.mjs # 364 pass / 0 fail +node bin/distilly.mjs doctor # Planned channels: collect reddit, collect gmail +``` + +测试覆盖:两页分页(游标串接 + 逐字字节 + 锚点正文 + 一页一条目)、429 退避毫秒数、 +缺凭据零写入、写动词被拒、Notion 页面 → 段落(跳过块点名)、Notion 白名单(两个 POST 放行、 +`POST /v1/pages` 与 `PATCH` 拒绝)、页面 id 从 URL 解析与坏 id 响亮失败、空页面不算空文档、 +kit 的脱敏与凭据查找(环境变量 / 配置文件 / legacy 路径 / 非法 JSON)。 + +## 5. 已知缺口 + +- **reddit / gmail 仍未实现**(各自需要什么写在 `PENDING_CHANNELS`,`doctor` 会列出)。 +- Discord 的线程/回复关系、Notion 的数据库行查询(白名单已放行 `POST + /v1/databases/{id}/query`,但 CLI 尚未暴露 `--database-id`)留待后续。 +- Discord 附件、Notion 的图片与嵌入块不anchored,按类型计入 warnings。 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-discord-notion.test.mjs b/tests/collect-discord-notion.test.mjs new file mode 100644 index 00000000..527e1ab6 --- /dev/null +++ b/tests/collect-discord-notion.test.mjs @@ -0,0 +1,279 @@ +/** + * The two channels added on top of the shared collector kit: Discord (bot token) + * and Notion (internal integration token). + * + * What is asserted is the contract every credentialed channel pays for: + * pagination that keeps raw bytes verbatim, a rate limit that backs off, a + * read-only allowlist that refuses before a socket opens, credentials that never + * reach a receipt, and a normalised text file the derivation can read. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import * as discord from "../src/collect/discord.mjs"; +import * as notion from "../src/collect/notion.mjs"; +import { assertReadOnly, loadCredential, redact, scrub } from "../src/collect/kit.mjs"; + +const SECRETS = { + discord: "FAKE-discord-bot-token-7c1f", + notion: "FAKE-notion-integration-token-4b9e", +}; + +function sandbox(channel) { + const root = mkdtempSync(join(tmpdir(), `dst-${channel}-`)); + const home = join(root, "distilly"); + mkdirSync(home, { recursive: true }); + return { + root, + home, + work: join(root, "work"), + env: { DISTILLY_HOME: home }, + config(values) { + writeFileSync(join(home, `${channel}_config.json`), `${JSON.stringify(values, null, 2)}\n`); + return this; + }, + read(rel) { + return readFileSync(join(this.work, rel), "utf8"); + }, + exists(rel) { + return existsSync(join(this.work, rel)); + }, + json(rel) { + return JSON.parse(this.read(rel)); + }, + }; +} + +const jsonResponse = (body, init = {}) => new Response(JSON.stringify(body), { status: init.status ?? 200, headers: init.headers ?? {} }); + +/* ------------------------------------------------------------------ discord */ + +function discordMessages(count, { start = Date.parse("2024-03-01T10:00:00Z"), author = { id: "u1", username: "林工" } } = {}) { + return Array.from({ length: count }, (_, index) => ({ + id: `m${String(index).padStart(3, "0")}`, + content: `第 ${index} 条:先看数据,再看日志。`, + timestamp: new Date(start - index * 60_000).toISOString(), + author, + })); +} + +test("discord: two pages, verbatim bytes, anchored text", async () => { + const box = sandbox("discord").config({ bot_token: SECRETS.discord }); + const pages = [discordMessages(3), discordMessages(2, { start: Date.parse("2024-03-01T09:50:00Z"), author: { id: "u2", username: "小明" } })]; + const calls = []; + let page = 0; + const fetchMock = async (url) => { + calls.push(String(url)); + return jsonResponse(pages[Math.min(page++, pages.length - 1)]); + }; + + const result = await discord.collect({ + fetch: fetchMock, + env: box.env, + root: box.work, + person: "demo", + channelId: "c1", + limit: 3, + now: "2026-09-13T00:00:00.000Z", + }); + + assert.equal(result.ok, true, JSON.stringify(result.receipt)); + assert.equal(result.receipt.pages, 2); + assert.equal(result.receipt.items, 5); + assert.ok(result.receipt.anchors.total >= 5); + assert.equal(calls.length, 2); + assert.equal(calls[0].includes("before="), false, "the first page carries no cursor"); + assert.equal(calls[1].includes("before=m002"), true, "the second page asks for messages before the oldest id"); + + // Raw bytes are what Discord sent, not a re-serialisation. + assert.equal(box.read("skills/colleague/demo/knowledge/raw/discord/c1-p001.json"), JSON.stringify(pages[0])); + const textDir = join(box.work, "skills", "colleague", "demo", "knowledge", "text"); + // Two pages, two documents: the second one is stemmed so it cannot overwrite the + // first (see the text-collision rule in `ledger.mjs`). Reading + // `readdirSync(textDir)[0]` made the assertion depend on directory order. + const files = readdirSync(textDir).sort(); + assert.equal(files.length, 2, `expected one text file per page, got ${files.join(", ")}`); + const body = files.map((name) => readFileSync(join(textDir, name), "utf8")).join("\n"); + assert.match(body, /^\[k0001\] 2024-03-01T10:00:00\.000Z 林工:第 0 条/m, body.slice(0, 200)); + assert.equal(body.includes("u1:"), false, "the REST author name is used, not the raw id"); + + const ledger = box.json("skills/colleague/demo/knowledge/index.json"); + assert.equal(ledger.length, 2, "one entry per page, each carrying raw and text"); + assert.equal(ledger[0].method, "api-bot-token"); + assert.equal(ledger[0].credentialed, true); + assert.equal(ledger[0].credential_file, "discord_config.json"); + assert.ok(ledger[0].locations.raw.startsWith("raw/discord/")); + assert.ok(ledger[0].locations.text.startsWith("text/")); +}); + +test("discord: 429 with retry_after seconds backs off and then succeeds", async () => { + const box = sandbox("discord").config({ bot_token: SECRETS.discord }); + const slept = []; + let attempt = 0; + const fetchMock = async () => { + attempt += 1; + if (attempt === 1) return jsonResponse({ message: "You are being rate limited.", retry_after: 1.5 }, { status: 429 }); + return jsonResponse(discordMessages(2)); + }; + + const result = await discord.collect({ + fetch: fetchMock, + env: box.env, + root: box.work, + person: "demo", + channelId: "c1", + sleep: async (ms) => slept.push(ms), + now: "2026-09-13T00:00:00.000Z", + }); + assert.equal(result.ok, true, JSON.stringify(result.receipt)); + assert.deepEqual(slept, [1500], "the body's retry_after is honoured in milliseconds"); + assert.match(result.receipt.warnings.join("\n"), /retry 1 after HTTP 429/); +}); + +test("discord: a missing credential fails loudly and writes nothing", async () => { + const box = sandbox("discord"); + const result = await discord.collect({ fetch: async () => jsonResponse([]), env: box.env, root: box.work, person: "demo", channelId: "c1" }); + assert.equal(result.ok, false); + assert.equal(result.exitCode, 1); + assert.match(result.receipt.unavailable[0].reason, /no credential/); + assert.equal(result.receipt.credential_file, "discord_config.json"); + assert.equal(box.exists("skills"), false); + assert.equal(JSON.stringify(result.receipt).includes(SECRETS.discord), false); +}); + +test("discord: mutating calls are refused before anything is sent", async () => { + const box = sandbox("discord").config({ bot_token: SECRETS.discord }); + let called = 0; + const fetchMock = async () => { + called += 1; + return jsonResponse([]); + }; + assert.throws( + () => assertReadOnly("https://discord.com/api/v10/channels/c1/messages/m1", "DELETE", discord.ALLOWED_CALLS, "discord"), + /read-only: DELETE/, + ); + const result = await discord.collect({ fetch: fetchMock, env: box.env, root: box.work, person: "demo", channelId: "c1" }); + assert.equal(result.ok, true); + assert.equal(called, 1, "only the read call was made"); +}); + +/* ------------------------------------------------------------------- notion */ + +const NOTION_PAGE = { + object: "page", + id: "1f2e3d4c-5b6a-7988-7766-554433221100", + properties: { Name: { type: "title", title: [{ plain_text: "事故复盘" }] } }, +}; + +function notionBlocks() { + return { + object: "list", + has_more: false, + next_cursor: null, + results: [ + { object: "block", id: "b1", type: "heading_1", heading_1: { rich_text: [{ plain_text: "时间线" }] } }, + { object: "block", id: "b2", type: "paragraph", paragraph: { rich_text: [{ plain_text: "02:10 告警,02:52 资金侧对平。" }] } }, + { object: "block", id: "b3", type: "to_do", to_do: { rich_text: [{ plain_text: "补幂等键。" }] } }, + { object: "block", id: "b4", type: "image", image: { file: { url: "https://example.invalid/x.png" } } }, + ], + }; +} + +test("notion: a page becomes anchored paragraphs, and skipped blocks are named", async () => { + const box = sandbox("notion").config({ integration_token: SECRETS.notion }); + const calls = []; + const fetchMock = async (url, init) => { + calls.push({ url: String(url), method: init.method }); + return String(url).includes("/pages/") ? jsonResponse(NOTION_PAGE) : jsonResponse(notionBlocks()); + }; + + const result = await notion.collect({ + fetch: fetchMock, + env: box.env, + root: box.work, + person: "demo", + pageId: "https://www.notion.so/Design-1f2e3d4c5b6a79887766554433221100?v=9", + now: "2026-09-13T00:00:00.000Z", + }); + + assert.equal(result.ok, true, JSON.stringify(result.receipt)); + assert.equal(result.receipt.title, "事故复盘"); + assert.equal(result.receipt.paragraphs, 3); + assert.equal(result.receipt.page_id, "1f2e3d4c-5b6a-7988-7766-554433221100"); + assert.equal(calls.every((call) => call.method === "GET"), true, "reading a page is GET-only"); + assert.match(result.receipt.warnings.join("\n"), /image×1/); + + const body = box.read("skills/colleague/demo/knowledge/text/notion.md"); + assert.match(body, /^\[k0002\] 时间线$/m); + assert.match(body, /^\[k0003\] 02:10 告警,02:52 资金侧对平。$/m); + const ledger = box.json("skills/colleague/demo/knowledge/index.json"); + assert.equal(ledger[0].method, "api-integration-token"); + assert.equal(ledger[0].credential_file, "notion_config.json"); + assert.equal(ledger[0].kind, "doc"); +}); + +test("notion: the search POST is allowed, anything else is not", () => { + assert.doesNotThrow(() => assertReadOnly("https://api.notion.com/v1/search", "POST", notion.ALLOWED_CALLS, "notion")); + assert.doesNotThrow(() => assertReadOnly("https://api.notion.com/v1/databases/abc/query", "POST", notion.ALLOWED_CALLS, "notion")); + assert.throws(() => assertReadOnly("https://api.notion.com/v1/pages", "POST", notion.ALLOWED_CALLS, "notion"), /read-only: POST \/v1\/pages/); + assert.throws(() => assertReadOnly("https://api.notion.com/v1/blocks/b1", "PATCH", notion.ALLOWED_CALLS, "notion"), /read-only: PATCH/); +}); + +test("notion: page ids are read out of urls and bad targets fail loudly", async () => { + assert.equal(notion.normalisePageId("1f2e3d4c5b6a79887766554433221100"), "1f2e3d4c-5b6a-7988-7766-554433221100"); + assert.equal(notion.normalisePageId("https://www.notion.so/x-1f2e3d4c5b6a79887766554433221100"), "1f2e3d4c-5b6a-7988-7766-554433221100"); + assert.throws(() => notion.normalisePageId("not a page"), /cannot read a Notion page id/); + + const box = sandbox("notion").config({ integration_token: SECRETS.notion }); + const result = await notion.collect({ fetch: async () => jsonResponse({}), env: box.env, root: box.work, person: "demo", pageId: "nope" }); + assert.equal(result.ok, false); + assert.match(result.receipt.errors[0], /cannot read a Notion page id/); + assert.equal(box.exists("skills"), false); +}); + +test("notion: an empty page is an error, never an empty document", async () => { + const box = sandbox("notion").config({ integration_token: SECRETS.notion }); + const fetchMock = async (url) => + String(url).includes("/pages/") ? jsonResponse(NOTION_PAGE) : jsonResponse({ object: "list", has_more: false, results: [] }); + const result = await notion.collect({ fetch: fetchMock, env: box.env, root: box.work, person: "demo", pageId: NOTION_PAGE.id }); + assert.equal(result.ok, false); + assert.equal(result.exitCode, 1); + assert.match(result.receipt.unavailable[0].reason, /no paragraph blocks/); + assert.equal(box.exists("skills/colleague/demo/knowledge/text"), false, "nothing is written for an empty page"); +}); + +/* --------------------------------------------------------------------- kit */ + +test("the kit keeps credentials out of receipts and refuses unknown verbs", () => { + assert.equal(redact(`token=${SECRETS.discord}`, [SECRETS.discord]), "token=***"); + const scrubbed = scrub({ nested: [{ note: SECRETS.notion }] }, [SECRETS.notion]); + assert.equal(JSON.stringify(scrubbed).includes(SECRETS.notion), false); + + const box = sandbox("discord"); + assert.throws(() => loadCredential({ env: box.env, configFile: "discord_config.json", envKeys: discord.ENV_KEYS, fields: ["bot_token"] }), /no credential/); + writeFileSync(join(box.home, "discord_config.json"), "{ not json\n"); + assert.throws(() => loadCredential({ env: box.env, configFile: "discord_config.json", envKeys: discord.ENV_KEYS, fields: ["bot_token"] }), /not valid JSON/); + + const fromEnv = loadCredential({ env: { DISTILLY_DISCORD_BOT_TOKEN: "abc" }, configFile: "discord_config.json", envKeys: discord.ENV_KEYS, fields: ["bot_token"] }); + assert.equal(fromEnv.source, "env"); + assert.equal(fromEnv.values.bot_token, "abc"); +}); + +test("both channels are on the CLI and no longer listed as pending", async () => { + const { PENDING_CHANNELS } = await import("../src/commands/credentialed.mjs"); + // `PENDING_CHANNELS` listed the channels that had no collector yet; discord and + // notion were the last two to leave it, and now every CONTRACT §1 channel ships, + // so the honest assertion is that nothing is waiting. A non-empty map means a + // channel regressed to "planned". + assert.deepEqual( + Object.keys(PENDING_CHANNELS).sort(), + [], + "every contract channel is implemented in this build", + ); + const cli = await import("../src/commands/credentialed.mjs"); + assert.equal(typeof cli, "object"); +}); 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/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", () => {