Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
53 changes: 53 additions & 0 deletions docs/evidence/pr-13-objective-audit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# PR-13 · 目标审计门禁

- 分支:直接落在集成分支 `dot-skill-test`(本地)
- 交付:`scripts/audit-objective.mjs`、`tests/audit-objective.test.mjs`、CI 两处步骤、
`docs/evidence/pr-01-node-core.md`、`docs/evidence/pr-02-parse-zero-cred.md`

## 1. 为什么

`acceptance.mjs` 证明"流水线能跑",但证明不了"范围是闭合的"。目标里的每条需求
(Node 单栈、证据脊柱、渲染与 visual-check、双语 prompt、宿主矩阵、渠道与同意、
schema v4、端到端验收、截图不入库、每个 PR 有证据文档)都应该有一条机械检查,
并且每条检查都要说清楚**它读到了什么**;缺口单独成行、不算失败。

## 2. 覆盖面(16 条)

Node 单栈(0 个 tracked `.py` + CI 无 Python 步骤)、证据脊柱磁盘契约与方括号锚点、
retrospect 确定性与锚点纪律、单文件离线页面 + CSP + visual-check、双语 prompt lint、
8 个宿主矩阵 + 防漂移测试、要 key 渠道 + computer-use 同意门(且浏览器一路不驱动浏览器)、
未移植渠道的记账、schema v4 + 幂等迁移、CONTRACT §1 每个命令可解析且 `PLANNED` 为空、
命令注册表规模、第二个公开语料存在且 CI 会跑它、证据图片不入库(项目素材除外)、
每个**真正合并过**的分支都有 PR 证据文档、端到端验收全绿、推送状态。

## 3. 写检查时踩的坑(全部修的是检查,不是结论)

| 误判 | 实际 |
| --- | --- |
| `agent.id` 取不到宿主机名 | `listAgents()` 返回的是**字符串** |
| "仓库不能有任何图片" | 项目素材(宿主 logo、社交预览)是合法的;要禁的是**证据**图片 |
| 把会话工作区的 `dst-evidence/` 当仓库内路径 | 它不在仓库里;仓库内只需断言"无证据图" |
| 用 `git branch --merged HEAD` 判断"已合并分支" | 当前特性分支的 tip 就是 HEAD,会被误判;改为按**合并提交**统计 |

## 4. 怎么验

```bash
node scripts/audit-objective.mjs # 16/16,2 条已知缺口
node scripts/audit-objective.mjs --skip-acceptance # 测试与 CI 的 test job 用这个
node --test tests/audit-objective.test.mjs # 2 条:全体满足 + 缺口必须显式
```

结果接进 `node --test` 与 CI 两个 job;`--json` 输出供其它脚本消费(`rows`/`failed`/`gaps`)。

## 5. 同轮补齐

- `docs/evidence/pr-01-node-core.md`、`pr-02-parse-zero-cred.md`:最早两条分支缺证据文档,
审计把它标成唯一未满足项后补写。
- 契约里未移植的四个采集渠道(discord/reddit/notion/gmail)从"读起来像拼错"改成
`collect/planned-channel` + 逐条需求说明,`doctor` 多打一行 `Planned channels`。

## 6. 已知缺口

- 四个渠道仍未实现(各自需要什么已写进 `PENDING_CHANNELS`)。
- 推送与 PR 受用户冻结影响,全部提交只在本地;解冻后的执行清单在
`dst-evidence/PR-BODIES/PR-PLAN.md`。
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
9 changes: 8 additions & 1 deletion scripts/acceptance.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
53 changes: 42 additions & 11 deletions scripts/audit-objective.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <ref>` → 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. */
{
Expand Down Expand Up @@ -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);
Expand All @@ -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 },
);
}

Expand Down
8 changes: 7 additions & 1 deletion src/collect/slack.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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/<file>` 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),
};
}

Expand Down
35 changes: 33 additions & 2 deletions tests/collect.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
Expand Down Expand Up @@ -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", () => {
Expand Down
23 changes: 21 additions & 2 deletions tests/feishu-mcp.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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/<file>`, 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. */
Expand Down Expand Up @@ -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: () => {},
});
Expand Down
57 changes: 47 additions & 10 deletions tests/package-payload.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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.
Expand Down Expand Up @@ -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", () => {
Expand Down
Loading