Commit c312dd9bdcaf651fdcf20d4fdbe9d2ee53a7f085
1 parent
be8de85b
docs(27-P3C): 纠正路径缺陷的归因 —— 不是三方争权威,是一处写错 + 一处死常量
并行会话指出我上一版归因错了,逐条复验后确认它是对的:
【纠正】runRootDir 与 companyRootDir 不是同一事实的两种拼法,是两个
不同的事实:
runRootDir → run 目录,覆盖态 {RUNS_ROOT}/{rootRunId},
扁平即可(rootRunId 全局唯一)
companyRootDir → 公司根,覆盖态 {RUNS_ROOT}/_companies/{cid},
那层 _companies 是必须的(公司 id 不是全局唯一坐标,
扁平放会和 run 目录撞名)
company-paths.js 的注释本身写着「理由与 runRootDir 那处 override
一致」,作者是有意区分的。我上一版写成「三种拼法争权威、实施前先定
哪个权威」,会让批 1 实施者去做一个根本不存在的设计决策。真正错的
只有一处:writeTaskGoalArtifact 自己拼路径。修法一行级。
【新增缺陷 B】goal-contract.js:75 声明了 RUNS_ROOT、注释还写着「与
graph-engine.RUNS_ROOT 是同一个变量、测试隔离在用」——然后全文件
一次都没用过,同文件的 resolveRunDir 照旧自己拼。而且 resolveRunDir
全仓库零调用方(只有定义、exports 和一行文档)。
它今天不会出错,危害是注释在撒谎:声明了意图、代码不兑现的常量比
没有这个常量更容易骗人,第一个接上去的人直接踩。批 1 一并处理 ——
要么真的用上,要么连函数带常量删掉(零调用方,删更省)。
【验收】12b 补上必须「先设 env 再 require」的姿势要求,12c 新增。
第六个实例登记:graph-engine:84 / goal-contract:75 / graph-run-sse:34
三处都在模块加载时快照 GRAPH_RUNS_DIR,而 company-paths:14 的注释
恰恰记着相反的教训,既有测试已在为此做变通。教训在一个模块里学会
了没传出去 —— 不是事实被复述,是「事实该怎么读取」的约定只传一半。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Showing
1 changed file
with
27 additions
and
7 deletions
docs/todo/system-design/27-p3c-goal-task-model.md
| @@ -485,19 +485,37 @@ for (const t of queuedTasks) { | @@ -485,19 +485,37 @@ for (const t of queuedTasks) { | ||
| 485 | 485 | ||
| 486 | 全部挂在 `server/routes/company.js`。错误响应沿用现有 `{ error, hint, ... }` 形态。 | 486 | 全部挂在 `server/routes/company.js`。错误响应沿用现有 `{ error, hint, ... }` 形态。 |
| 487 | 487 | ||
| 488 | -> **⚠️ 批 1 必须修一处路径分叉 —— 它就在本文档的核心链路上。** | 488 | +> **⚠️ 批 1 必须修两处路径缺陷 —— 一处就在本文档的核心链路上。** |
| 489 | > | 489 | > |
| 490 | -> `task-goal.json` 的**写入方与读取方用的不是同一套路径规则**: | 490 | +> **缺陷 A(会分叉,在核心链路上)**:`task-goal.json` 的写入方与读取方用的不是同一套路径规则。 |
| 491 | > | 491 | > |
| 492 | > | | 位置 | 认不认 `GRAPH_RUNS_DIR` | | 492 | > | | 位置 | 认不认 `GRAPH_RUNS_DIR` | |
| 493 | > |---|---|---| | 493 | > |---|---|---| |
| 494 | > | **写** | `company.js` 的 `writeTaskGoalArtifact` → `path.resolve(__dirname,'../..','data','companies',cid,'runs',rid,'artifacts')` | ❌ **不认** | | 494 | > | **写** | `company.js` 的 `writeTaskGoalArtifact` → `path.resolve(__dirname,'../..','data','companies',cid,'runs',rid,'artifacts')` | ❌ **不认** | |
| 495 | -> | **读** | `graph-engine.js` 的 `runRootDir`(`describeTask` 与 `goal-contract` 都走它)→ `RUNS_ROOT ? path.join(RUNS_ROOT, root_run_id) : …` | ✅ 认 | | ||
| 496 | -> | 第三种 | `company-paths.js` 的 `companyRootDir` → `runsRoot ? path.join(runsRoot,'_companies',cid) : …` | ✅ 认,但**形状又不一样** | | 495 | +> | **读** | `graph-engine.js` 的 `runRootDir`(`describeTask` 与 `goal-contract.deriveRunDir` 都走它)| ✅ 认 | |
| 497 | > | 496 | > |
| 498 | -> 同一个事实**三种拼法**。`GRAPH_RUNS_DIR` 一旦被设上(目前只有测试在用),写进 A 处、从 B 处读 —— `describeTask` 拿到 null,**静默退回兜底文案「执行图节点任务」**,agent 从此拿不到真实任务描述,而**没有任何报错**。`goal-contract.test.js` 的注释可以佐证读取方那套形状才是约定:`{GRAPH_RUNS_DIR}/{rootRunId}/artifacts/task-goal.json`。 | 497 | +> `GRAPH_RUNS_DIR` 一旦被设上(目前只有测试在用),写进 A 处、从 B 处读 —— `describeTask` 拿到 null,**静默退回兜底文案「执行图节点任务」**,agent 从此拿不到真实任务描述,而**没有任何报错**。`goal-contract.test.js` 的注释佐证读取方那套形状才是约定:`{GRAPH_RUNS_DIR}/{rootRunId}/artifacts/task-goal.json`。 |
| 499 | > | 498 | > |
| 500 | -> 生产环境两边恰好都退到同一个默认分支,所以今天不发作 —— 但**批 1 会把 artifact 的落盘点从「下达时」前移到「入队时」**(§四.3 / §四.4 的描述冻结点),正好要动这个函数。**修的时机就是那时**:让写入方复用读取方的同一个 helper(`runRootDir` 提到 `company-paths` 或直接导出),三种拼法收敛成一种。 | 499 | +> 生产环境两边恰好都退到同一个默认分支,所以今天不发作 —— 但**批 1 会把 artifact 的落盘点从「下达时」前移到「入队时」**(§四.3 / §四.4 的描述冻结点),正好要动这个函数。**修法是一行级:写侧改调读侧的 `runRootDir`。** |
| 500 | +> | ||
| 501 | +> > ⚠️ **本文档初稿在这里归因错了,留档纠正**:初稿写成「`runRootDir` / `companyRootDir` / 写入方三种拼法争权威,实施前得先定哪个权威」。**那是错的,而且会让实施者去做一个根本不存在的设计决策。** `runRootDir` 与 `companyRootDir` 是**两个不同的事实**,不是同一事实的两种拼法: | ||
| 502 | +> > - `runRootDir` → **run 目录**,覆盖态是 `{RUNS_ROOT}/{rootRunId}`。扁平即可,因为 `rootRunId` 全局唯一; | ||
| 503 | +> > - `companyRootDir` → **公司根**,覆盖态是 `{RUNS_ROOT}/_companies/{cid}`。多出来的 `_companies/` **是必须的** —— 公司 id 不是全局唯一坐标,扁平放会和 run 目录撞名。 | ||
| 504 | +> > | ||
| 505 | +> > `company-paths.js` 的注释本身就写着「理由与 `runRootDir` 那处 override 一致」,作者是有意区分的。**真正错的只有一处:写入方自己拼路径。** | ||
| 506 | +> | ||
| 507 | +> **缺陷 B(不会分叉,因为它根本没有调用方 —— 但更会骗人)**:`goal-contract.js:75` 声明了 | ||
| 508 | +> | ||
| 509 | +> ```js | ||
| 510 | +> /** run 目录根的覆盖,与 graph-engine.RUNS_ROOT / graph-run-sse 是同一个变量(测试隔离在用) */ | ||
| 511 | +> const RUNS_ROOT = process.env.GRAPH_RUNS_DIR || null; | ||
| 512 | +> ``` | ||
| 513 | +> | ||
| 514 | +> **然后一次都没用过**(全文件只有注释 + 声明两行),同文件的 `resolveRunDir()` 照旧自己拼 `DATA_DIR/companies/…`。而且 `resolveRunDir` **全仓库零调用方**(只有定义、`module.exports` 和 `27-goal-contract-format.md` 里的一行文档)—— 又一个「实现在、零调用方」。 | ||
| 515 | +> | ||
| 516 | +> 所以它今天不会出错,危害是**注释在撒谎**:白纸黑字写着「与 graph-engine.RUNS_ROOT 是同一个变量、测试隔离在用」,读代码的人会据此认为这个文件认 `GRAPH_RUNS_DIR`。**一个声明了意图、代码却不兑现的常量,比没有这个常量更容易骗人** —— 第一个把 `resolveRunDir` 接上去的人会直接踩。 | ||
| 517 | +> | ||
| 518 | +> 批 1 一并处理:要么让它真的用 `RUNS_ROOT`,要么连函数带常量一起删(零调用方,删是更省的一侧)。 | ||
| 501 | > | 519 | > |
| 502 | > **`system/agents` 那一族已经不用管了** —— 2026-08-18 的 `e120645` 已把全仓库 13 处(跨 5 个文件,含 `company.js` 里 6 个 `TEMPLATES_DIR` + 3 个同义异名的 `DATA_DIR`)收敛到 `locator.AGENTS_ROOT`,并新增 `server/tests/unit/agents-root-single-holder.test.js` 锁住不变式。**批 1 新增的代码必须遵守它**:不许自己拼「以 `../` 开头、以 `system/agents` 结尾」的字面量,否则这条测试会红。 | 520 | > **`system/agents` 那一族已经不用管了** —— 2026-08-18 的 `e120645` 已把全仓库 13 处(跨 5 个文件,含 `company.js` 里 6 个 `TEMPLATES_DIR` + 3 个同义异名的 `DATA_DIR`)收敛到 `locator.AGENTS_ROOT`,并新增 `server/tests/unit/agents-root-single-holder.test.js` 锁住不变式。**批 1 新增的代码必须遵守它**:不许自己拼「以 `../` 开头、以 `system/agents` 结尾」的字面量,否则这条测试会红。 |
| 503 | > | 521 | > |
| @@ -614,7 +632,9 @@ for (const t of queuedTasks) { | @@ -614,7 +632,9 @@ for (const t of queuedTasks) { | ||
| 614 | 10. 列表端点在 20 goal × 3 task 的公司上**不 N+1**(`graph_run_nodes` 的进度用一次 `GROUP BY` 出)。 | 632 | 10. 列表端点在 20 goal × 3 task 的公司上**不 N+1**(`graph_run_nodes` 的进度用一次 `GROUP BY` 出)。 |
| 615 | 11. `/api/goals` 与 `/api/company/:cid/goals` 的 OKR 别名退役后,`OkrDashboard` / `CompanyDashboard` / `bin/lemon.js` 全部功能正常(它们走的是 `/api/okr`)。 | 633 | 11. `/api/goals` 与 `/api/company/:cid/goals` 的 OKR 别名退役后,`OkrDashboard` / `CompanyDashboard` / `bin/lemon.js` 全部功能正常(它们走的是 `/api/okr`)。 |
| 616 | 12. **回填**:跑迁移脚本后,既存的每条 root run 在左栏都以 goal + task 的形式出现,描述从 `task-goal.json` 读回(§四.8)。**这条必须在批 1 就验** —— 不验就等于批 1 让历史任务更看不见了。 | 634 | 12. **回填**:跑迁移脚本后,既存的每条 root run 在左栏都以 goal + task 的形式出现,描述从 `task-goal.json` 读回(§四.8)。**这条必须在批 1 就验** —— 不验就等于批 1 让历史任务更看不见了。 |
| 617 | -12b. **路径分叉已修**:在 `GRAPH_RUNS_DIR` 指向临时目录的条件下走一遍「下达 → 入队 → 读回描述」,`describeTask` 拿到的是**真实任务描述**而不是兜底文案「执行图节点任务」(验 §六 那处三种拼法已收敛为一种)。 | 635 | +12b. **路径缺陷 A 已修**:在 `GRAPH_RUNS_DIR` 指向临时目录的条件下走一遍「下达 → 入队 → 读回描述」,`describeTask` 拿到的是**真实任务描述**而不是兜底文案「执行图节点任务」。 |
| 636 | +> ⚠️ 这条验收**必须先设 env 再 `require`**:`graph-engine.js:84` / `goal-contract.js:75` / `graph-run-sse.js:34` 三处都是**模块加载时快照** `GRAPH_RUNS_DIR`,而 `company-paths.js:14` 的文件头注释恰恰记着相反的教训(「每次调用时读,不在模块加载时快照 —— 镜像时代两个模块各自快照,加载顺序不同就会算出不同的根,测试先 require 再设 env 时尤其明显」)。既有测试已经在为此做变通(`graph-engine.test.js` 注释:「`graph-run-sse` 在模块顶部快照 `GRAPH_RUNS_DIR`,必须连它一起重新 require」)。**教训在一个模块里学会了,没传出去** —— 这是本清单的第六个实例,形态还不一样:不是事实被复述,是**「事实该怎么读取」的约定只传了一半**。 | ||
| 637 | +12c. **路径缺陷 B 已处理**:`goal-contract.js` 里那个声明了却从未使用的 `RUNS_ROOT` 要么真的接上、要么连 `resolveRunDir` 一起删;不留「注释承诺了、代码不兑现」的常量。 | ||
| 618 | 13. **停止即出队**:`max=1`,任务 A 跑着、B 排队,对 A 点「停止」→ **B 在几秒内自动起跑**(验证 §五.1.1 的漏斗钩子覆盖了 `stopRun` 这条不走 `settleRun` 的路径),不是等 60s 兜底。 | 638 | 13. **停止即出队**:`max=1`,任务 A 跑着、B 排队,对 A 点「停止」→ **B 在几秒内自动起跑**(验证 §五.1.1 的漏斗钩子覆盖了 `stopRun` 这条不走 `settleRun` 的路径),不是等 60s 兜底。 |
| 619 | 14. **预算耗尽即出队**:把 A 的预算调到极小让它烧成 `blocked` → B 同样自动起跑。 | 639 | 14. **预算耗尽即出队**:把 A 的预算调到极小让它烧成 `blocked` → B 同样自动起跑。 |
| 620 | 15. **排队期间 replan**:B 排队时点「重新规划组织」改动 org graph → B 出队后**按新图跑**,没有孤儿节点、没有卡死(验证 `syncRunNodesToDef`)。 | 640 | 15. **排队期间 replan**:B 排队时点「重新规划组织」改动 org graph → B 出队后**按新图跑**,没有孤儿节点、没有卡死(验证 `syncRunNodesToDef`)。 |