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
115 changes: 115 additions & 0 deletions docs/evidence/pr-20-repo-hygiene.md
Original file line number Diff line number Diff line change
@@ -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` 约束)。
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
Loading
Loading