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
61 changes: 61 additions & 0 deletions docs/evidence/pr-19-release-migration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# PR-19 · 迁移把旧内容导入证据脊柱 + 发布检查

- 分支:`dot-skill-test`(本地集成分支)
- 交付:`src/skill/migrate.mjs`(legacy 导入)、`src/commands/{migrate,harvest}.mjs`、
`scripts/check_release.mjs`(新)、`tests/{schema-migration,release-check}.test.mjs`、CI、审计

## 1. 迁移的缺口(做发布检查时发现的真实缺陷)

`skill migrate` 原来只做三件事:建 v4 目录、写一个**空**账本、改 `schema_version`。
v3 skill 的材料在 `knowledge/{docs,messages,emails}/` 里——迁移之后它们仍在原地,
账本 0 条、`knowledge/text/` 是空的,**派生层依然看不到任何证据**。也就是说"迁移成功"
只体现在目录形状上。

现在迁移会把旧内容导入证据脊柱(复用 `harvest` 的解析分发,`documentFor` 改为导出):

| 指标 | 改动前 | 改动后 |
| --- | --- | --- |
| 账本条目 | 0 | **2**(`k0001` / `k0002`,各带 `locations.raw` + `locations.text`) |
| `knowledge/text/` | 空 | 两份带 `[k0001]` 锚点的正文(消息日志保留 `老周:` 前缀) |
| `retrospect` | 无输入 | 产出 7 个派生文件(3 条可引用段落,低于最低样本 → 如实给"样本不足") |
| 第二次迁移 | 0 changed | 0 changed(幂等) |
| 旧文件 | 原地 | **原地保留,不删不改**(指纹比对断言) |

细节:`origin` 用旧文件的路径,所以重复迁移按 sha256+origin 去重;
解析不了的文件进 `skipped` 并在 actions 里点名(不静默);`migrateSkillDir` 的返回值
补齐 `imported: {files, entries, skipped}`——"已是最新"的早返回过去缺这个字段,
调用方得特判。

## 2. 发布检查(STATUS 里承诺过、一直缺席)

`scripts/check_release.mjs`:7 项发布卫生检查,每项都写出它读到了什么。

| # | 检查 | 现状 |
| --- | --- | --- |
| 1 | 版本三处一致(`package.json` / `--version` / `SKILL.md`);`--tag` 时校验 tag | 1.0.0 / 1.0.0 / 1.0.0 |
| 2 | `SCHEMA_VERSION=4` + 账本 v2 + 迁移脚本在 + 幂等断言 | ✅ |
| 3 | 安装器携带 evidence spine(`knowledge/raw`、`knowledge/text`、`evidence`、`views`) | ✅ |
| 4 | 零运行时依赖 + 无 Python 残留 | dependencies 0 / tracked .py 0 |
| 5 | 生成物与源同步(模板 `--check` + 拼音表在) | ✅ |
| 6 | 门禁齐备且 CI 会跑(`node --test` / acceptance / prompt-lint / audit) | ✅ |
| 7 | 发布的文档都在(CONTRACT / ACCEPTANCE / STATUS / MIGRATION / IDENTITY / README) | 6/6 |

接进 CI(test job)与审计(16 → **17** 条),并有 2 条测试:全过;以及
`--tag v9.9.9` **必须失败**(证明它真的会红)。

## 3. 怎么验

```bash
node scripts/check_release.mjs # 7/7
node --test tests/*.test.mjs # 378 pass / 0 fail
node scripts/audit-objective.mjs --skip-acceptance # 17/17
node bin/distilly.mjs skill migrate --base-dir <v3-skill> # 导入旧内容,跑两次幂等
```

命令与产物原文:`dst-evidence/screenshots/pr-19-release-migration/migration-and-release.txt`。

## 4. 已知缺口

- 发布检查不校验 CHANGELOG(仓库没有该文件;要发版时再决定是否引入)。
- 迁移导入的是"文件形态"的旧内容;v3 时代若把材料直接写在 `persona.md`/`work.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