宜博 / lemonboss

Commit 20fed0a5673f8ee29fc431c5f5c82fc82e768afe

Authored by yibo
1 parent c3a5d462

fix(test-isolation): 测试不再往生产图库写，并加运行期闸门堵住这一类 

## 病症

agent-chat-tenant-isolation.test.js 每跑一次，往共享生产库 data/graph.sqlite
落 2 条 goal + 2 条 task。实测 29→31。库里 31 条 goal 里 **22 条是它的残留**，
测试期间一直全绿。

根因是批 5 的存储迁移留下的半截清理：/task/dispatch 的元数据从
system/tasklog/*.json 换进了 tasks 表，而这个用例的 afterAll 只会清那个目录 ——
清理代码还在、清的东西已经不在那儿了。

## 这是第四次，但换了个方向进来

前三次（BENCHMARK_DB_PATH / USERS_DB_PATH / SKILLS_DIR，记在
test-isolation-env.test.js 文件头）都是「设了 env，生产代码不读」。
那条守卫查的正是这个形态。

这次是**一个字都没设** —— 没有可查的东西，守卫自然查不出来。
静态扫源码也判不出「这个测试会不会开库」（要跨 require 链推导）。

## 修法

1. 用例自己开一次性库（fs.mkdtempSync + GRAPH_DB_PATH），afterAll 连目录删掉。
   赋值必须在 require 路由**之前** —— graph-db.js 的 DB_PATH 是模块级 const。
2. graph-db.getDb() 加运行期闸门：VITEST 为真且没设 GRAPH_DB_PATH → 抛，
   报错里点名 env、点明「要在 require 之前设」。
   放在 getDb() 不放模块顶层：只为取常量而 require 是合法用法
   （goal-task-model.test.js 就这么用），顶层抛会误伤。真正开库才是越界那一刻。
3. 守卫 graph-db-prod-guard.test.js，4 例：撞库要抛且报错可照做、设了 env 要能开、
   没有 VITEST 时不生效（防止把生产一起锁死）、样本非空。
   反向验过：注释掉闸门那一行，第 1 例立刻红。

## 顺带修一条 flaky（我自己上一版留下的）

agent-docs-port.test.js 先 walk 出文件清单再逐个读，中间别的用例把 zz-* 夹具的
运行态目录清掉 → ENOENT。**报错长得像端口违例，其实跟端口无关**，实测 8 次跑中 2 次红。
只放过 ENOENT 一种（文件没了），其它读错误照抛 —— 一律 try/catch 吞掉的话，
权限错、编码错会被当成「无违例」。

## 存量清理

删掉生产库里 created_by LIKE 'zz-tenant-%' 的 22 goal + 22 task
（备份 /tmp/graph.sqlite.bak-c3a5d46）。零 run 引用它们。
其余 9 条 goal 是真实数据，一条没动。

全量：136 文件 / 1276 测试，连跑 8 次全绿（改之前 8 次里红 2 次）。
跑完后生产库行数零增量。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Inline Side-by-side
Showing 4 changed files with 145 additions and 13 deletions
server/services/graph-db.js
  View file @20fed0a
... ... @@ -216,8 +216,33 @@ function createEdgesTable(database) {
216 216 `);
217 217 }
218 218  
  219 +/**
  220 + * 测试进程打开**共享生产库**时直接拦下来。
  221 + *
  222 + * 挡的是「测试压根没设隔离 env」这个形态 —— `test-isolation-env.test.js` 挡不住它：
  223 + * 那条守卫查的是「设了 XXX_PATH、生产代码却不读」，一个字都没设的文件没有可查的东西。
  224 + * 实际后果：`agent-chat-tenant-isolation.test.js` 每跑一次往 `data/graph.sqlite` 落
  225 + * 2 条 goal + 2 条 task，攒到 25 条 `_global` goal 才被人翻库翻出来，期间测试一直全绿。
  226 + *
  227 + * 为什么放在 getDb() 而不是模块顶层：`require('work-graph-db')` 只为取一个常量是合法的
  228 + * （goal-task-model.test.js 就这么用），顶层抛会误伤。**真正开库**才是越界那一刻。
  229 + *
  230 + * 需要读真库的测试显式写 `process.env.GRAPH_DB_PATH = <真库路径>` 表明意图即可。
  231 + */
  232 +function assertNotProdDbInTests() {
  233 + if (!process.env.VITEST) return;
  234 + if (process.env.GRAPH_DB_PATH) return;
  235 + throw new Error(
  236 + `[graph-db] 测试进程试图打开共享生产库 ${DB_PATH}。\n` +
  237 + `请在 require 任何路由/服务**之前**设 process.env.GRAPH_DB_PATH 指向一次性库` +
  238 + `（fs.mkdtempSync 造目录），afterAll 里删掉。\n` +
  239 + `DB_PATH 是模块级 const，晚一步赋值就已经绑到生产库上了。`
  240 + );
  241 +}
  242 +
219 243 function getDb() {
220 244 if (db) return db;
  245 + assertNotProdDbInTests();
221 246  
222 247 if (!fs.existsSync(DATA_DIR)) {
223 248 fs.mkdirSync(DATA_DIR, { recursive: true });
... ...
server/tests/unit/agent-chat-tenant-isolation.test.js
  View file @20fed0a
... ... @@ -24,12 +24,23 @@
24 24 * 顺带治了一个 flaky：本体被建了又删时，别的用例正在遍历整棵 `system/agents/`，
25 25 * 撞上「列到了名字、进去时已经没了」→ `ENOENT: scandir system/agents/zz-tenant-boss`。
26 26 * 不用 `_` 前缀：那是 `_global` / `_base` 的保留段，会被 isReservedCompanyId 认成非公司项。
  27 + *
  28 + * **图库必须另开一份**（下面 GRAPH_DB_PATH）。批 5 把 `/task/dispatch` 的存储从
  29 + * `system/tasklog/*.json` 换进了 `tasks` 表，而 afterAll 当时只会清那个目录 ——
  30 + * 于是本文件每跑一次就往**共享的生产库** `data/graph.sqlite` 落 2 条 goal + 2 条 task，
  31 + * 测试照样全绿。实测残留 25 条 `_global` goal 才被发现。
  32 + *
  33 + * 这是本仓库第四次踩「测试污染共享库」（前三次 BENCHMARK_DB_PATH / USERS_DB_PATH /
  34 + * SKILLS_DIR，记在 test-isolation-env.test.js 的文件头），但换了个方向进来：
  35 + * 前三次是「设了 env、生产代码不读」，这次是**压根没设 env** —— 那条守卫查的是前者，
  36 + * 没有可查的东西就查不出来。现由 graph-db.getDb() 的运行期闸门兜底（见该函数）。
27 37 */
28 38 process.env.LEMONBOSS_CLAUDE_MODE = 'mock'; // 必须在 require 路由之前：acp-core 的 mock 开关是模块级常量
29 39  
30 40 import { describe, it, expect, beforeAll, afterAll } from 'vitest';
31 41 import fs from 'fs';
32 42 import path from 'path';
  43 +import os from 'os';
33 44 import express from 'express';
34 45 import { createRequire } from 'node:module';
35 46  
... ... @@ -37,11 +48,15 @@ const require_ = createRequire(import.meta.url);
37 48 const ROOT = path.resolve(__dirname, '../../..');
38 49 const AGENTS_ROOT = path.join(ROOT, 'system', 'agents');
39 50  
40   -// 任务**文本**带一次性 nonce（不是 id）：/task/dispatch 有 30 分钟去重窗口，元数据落在
41   -// system/tasklog/ 且跨进程存活，文本固定的话第二次跑本文件会被 409 拦掉。
  51 +// 一次性图库 —— 必须在 require 路由**之前**赋值：graph-db.js 的 DB_PATH 是模块级 const，
  52 +// 晚一步就已经绑到生产库上了。目录用 mkdtempSync，不用时间戳拼名（见 test-id-stability）。
  53 +const TMP_DB_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'lemonboss-tenant-iso-'));
  54 +process.env.GRAPH_DB_PATH = path.join(TMP_DB_DIR, 'graph.sqlite');
  55 +
  56 +// 任务**文本**带一次性 nonce（不是 id）：/task/dispatch 有 30 分钟去重窗口，文本固定的话
  57 +// 同一次跑里第二条会被 409 拦掉。（跨次跑已经不会撞了 —— 库是一次性的。）
42 58 // 它拼的是自然语言，不进任何注册表，所以与「id 必须固定」那条规矩不冲突（见 test-id-stability）。
43 59 const RUN = `r${Date.now().toString(36)}`;
44   -const TASKLOG_DIR = path.join(ROOT, 'system', 'tasklog');
45 60  
46 61 const CEO = 'zz-tenant-ceo'; // 被两家公司共用的那一份本体
47 62 const BOSS = 'zz-tenant-boss'; // 派单方
... ... @@ -103,14 +118,9 @@ describe(&#39;agent-chat · 公司段不能丢（todo/plan 落点、文件写入根
103 118 for (const cid of [ALPHA, BETA]) {
104 119 fs.rmSync(path.join(ROOT, 'data', 'companies', cid), { recursive: true, force: true });
105 120 }
106   - // /task/dispatch 的元数据落在本体目录之外，得单独收：只删 to 指向本用例 agent 的那几条
107   - for (const f of fs.existsSync(TASKLOG_DIR) ? fs.readdirSync(TASKLOG_DIR) : []) {
108   - if (!f.endsWith('.json')) continue;
109   - try {
110   - const tm = JSON.parse(fs.readFileSync(path.join(TASKLOG_DIR, f), 'utf-8'));
111   - if (String(tm.to || '').includes('zz-tenant-')) fs.rmSync(path.join(TASKLOG_DIR, f), { force: true });
112   - } catch {}
113   - }
  121 + // /task/dispatch 的 goal/task 行落在图库里，整份库是一次性的 —— 连目录一起删掉即可，
  122 + // 不需要再按 to 字段挑行（那是 tasklog 时代的清理法，存储换进表之后就失效了）。
  123 + fs.rmSync(TMP_DB_DIR, { recursive: true, force: true });
114 124 });
115 125  
116 126 const dispatch = (cid, task) =>
... ...
server/tests/unit/agent-docs-port.test.js
  View file @20fed0a
... ... @@ -28,7 +28,15 @@ const SKIP_DIR = /(^|\/)(node_modules|\.git|dist|coverage|venv|\.venv|site-packa
28 28  
29 29 function walk(dir, out = []) {
30 30 if (!fs.existsSync(dir) || SKIP_DIR.test(dir)) return out;
31   - for (const e of fs.readdirSync(dir, { withFileTypes: true })) {
  31 + // existsSync 与 readdirSync 之间同样有窗口（理由见下面读文件那处）——存在性检查不是锁。
  32 + let entries;
  33 + try {
  34 + entries = fs.readdirSync(dir, { withFileTypes: true });
  35 + } catch (e) {
  36 + if (e.code === 'ENOENT') return out;
  37 + throw e;
  38 + }
  39 + for (const e of entries) {
32 40 const full = path.join(dir, e.name);
33 41 if (e.isDirectory()) walk(full, out);
34 42 else if (/\.(md|json|txt)$/.test(e.name)) out.push(full);
... ... @@ -48,7 +56,16 @@ describe(&#39;agent 提示词里的端口&#39;, () =&gt; {
48 56 it('不许再出现 localhost:3000（实际端口是 3010）', () => {
49 57 const offenders = [];
50 58 for (const f of files) {
51   - const src = fs.readFileSync(f, 'utf-8');
  59 + // 扫的是活目录：walk 列出名字之后、读到它之前，别的用例可能正好把自己的运行态
  60 + // 目录清掉（`zz-*` 那几个夹具就是这么干的）→ ENOENT。**只放过「文件没了」这一种**，
  61 + // 其它读错误照抛 —— 一律 try/catch 吞掉的话，权限错、编码错也会被当成「无违例」。
  62 + let src;
  63 + try {
  64 + src = fs.readFileSync(f, 'utf-8');
  65 + } catch (e) {
  66 + if (e.code === 'ENOENT') continue;
  67 + throw e;
  68 + }
52 69 src.split('\n').forEach((line, i) => {
53 70 if (line.includes('localhost:3000') || line.includes('127.0.0.1:3000')) {
54 71 offenders.push(`${path.relative(ROOT, f)}:${i + 1} → ${line.trim().slice(0, 90)}`);
... ...
server/tests/unit/graph-db-prod-guard.test.js 0 → 100644
  View file @20fed0a
  1 +/**
  2 + * Unit Tests — **测试进程不许打开共享生产库 `data/graph.sqlite`**
  3 + *
  4 + * 守的是 `graph-db.getDb()` 里的 `assertNotProdDbInTests()`。
  5 + *
  6 + * ── 为什么再加一条 ─────────────────────────────────────────────────────
  7 + * `test-isolation-env.test.js` 已经守了同一族缺陷，但它的判据是
  8 + * 「测试设了 XXX_PATH / XXX_DIR，生产代码必须真的读它」—— 那挡的是**设了但没接通**。
  9 + * 这次漏进来的是另一个形态：`agent-chat-tenant-isolation.test.js` **一个字都没设**，
  10 + * 于是那条守卫没有可查的东西，而 `/task/dispatch` 每跑一次往生产库落
  11 + * 2 条 goal + 2 条 task。攒到 25 条 `_global` goal 才被人翻库发现，期间测试一直全绿。
  12 + *
  13 + * 静态扫源码判不出「这个测试会不会开库」（要跨 require 链推导），所以这条守卫改成
  14 + * **运行期闸门 + 对它本身的黑盒验证**：起一个真子进程去撞，看撞不撞得动。
  15 + *
  16 + * ── 判据 ───────────────────────────────────────────────────────────────
  17 + * 只在 `process.env.VITEST` 为真时生效，只在**真正开库那一刻**判（不是 require 时）——
  18 + * 只为取一个常量而 require 本模块是合法用法，顶层抛会误伤（goal-task-model.test.js 就这么用）。
  19 + */
  20 +import { describe, it, expect } from 'vitest';
  21 +import { execFileSync } from 'child_process';
  22 +import fs from 'fs';
  23 +import os from 'os';
  24 +import path from 'path';
  25 +
  26 +const ROOT = path.resolve(__dirname, '../../..');
  27 +const REQUIRE_DB = `require(${JSON.stringify(path.join(ROOT, 'server/services/graph-db.js'))})`;
  28 +
  29 +/** 在干净子进程里跑一小段，返回 {code, stderr}。不继承本进程的 VITEST / GRAPH_DB_PATH。 */
  30 +function run(code, env) {
  31 + const base = { ...process.env };
  32 + delete base.VITEST;
  33 + delete base.GRAPH_DB_PATH;
  34 + try {
  35 + execFileSync(process.execPath, ['-e', code], {
  36 + env: { ...base, ...env },
  37 + stdio: ['ignore', 'pipe', 'pipe'],
  38 + cwd: ROOT,
  39 + });
  40 + return { code: 0, stderr: '' };
  41 + } catch (e) {
  42 + return { code: e.status, stderr: String(e.stderr || '') };
  43 + }
  44 +}
  45 +
  46 +describe('graph-db · 测试进程不许开生产库', () => {
  47 + it('VITEST=1 且没设 GRAPH_DB_PATH → getDb() 抛，且报错里给得出修法', () => {
  48 + const r = run(`${REQUIRE_DB}.getDb()`, { VITEST: '1' });
  49 + expect(r.code).not.toBe(0);
  50 + expect(r.stderr).toContain('测试进程试图打开共享生产库');
  51 + // 光抛不够 —— 撞上的人要能照着改，所以报错必须点名那个 env 和「要在 require 之前设」
  52 + expect(r.stderr).toContain('GRAPH_DB_PATH');
  53 + expect(r.stderr).toContain('require');
  54 + });
  55 +
  56 + it('VITEST=1 但设了 GRAPH_DB_PATH → 正常开一次性库（判据不是「测试里一律不许开库」）', () => {
  57 + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'lemonboss-graphdb-guard-'));
  58 + const dbPath = path.join(dir, 'graph.sqlite');
  59 + try {
  60 + const r = run(`${REQUIRE_DB}.getDb()`, { VITEST: '1', GRAPH_DB_PATH: dbPath });
  61 + expect(r.stderr).toBe('');
  62 + expect(r.code).toBe(0);
  63 + expect(fs.existsSync(dbPath)).toBe(true);
  64 + } finally {
  65 + fs.rmSync(dir, { recursive: true, force: true });
  66 + }
  67 + });
  68 +
  69 + it('不在测试进程里（无 VITEST）→ 闸门不生效，生产照常开库', () => {
  70 + // 这一条防的是把闸门写死成「永远不许开 data/graph.sqlite」——那会把生产也一起锁死。
  71 + const r = run(`${REQUIRE_DB}.getDb(); console.log('opened')`, {});
  72 + expect(r.code).toBe(0);
  73 + });
  74 +
  75 + it('样本非空：这个仓库里真的存在会开库的路径', () => {
  76 + // 少了这条，上面三条在「模块路径写错、require 直接失败」时也会以「抛了异常」的形式变绿。
  77 + const mod = require('../../services/graph-db.js');
  78 + expect(typeof mod.getDb).toBe('function');
  79 + });
  80 +});
... ...