From 3d1ea7a4032b26079bed59cec4fcf5b0b87f8817 Mon Sep 17 00:00:00 2001 From: graphlemon Date: Tue, 4 Aug 2026 16:33:31 +0800 Subject: [PATCH] graphlemon: brd artifact for task fe8e1b3d-dea1-4fa1-aa53-8cdd77d977f3 --- .graphlemon/runs/fe8e1b3d-dea1-4fa1-aa53-8cdd77d977f3/brd.md | 155 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+), 0 deletions(-) create mode 100644 .graphlemon/runs/fe8e1b3d-dea1-4fa1-aa53-8cdd77d977f3/brd.md diff --git a/.graphlemon/runs/fe8e1b3d-dea1-4fa1-aa53-8cdd77d977f3/brd.md b/.graphlemon/runs/fe8e1b3d-dea1-4fa1-aa53-8cdd77d977f3/brd.md new file mode 100644 index 0000000..9ed33e5 --- /dev/null +++ b/.graphlemon/runs/fe8e1b3d-dea1-4fa1-aa53-8cdd77d977f3/brd.md @@ -0,0 +1,155 @@ +> **NOTE**: 依据 `SOUL.md` §16(迁移后的状态白名单约束),本 Agent 不直接改写 `task.status`,仅产出本 BRD Artifact 供 `c_org_node_trigger` 判定流转;下文的证据与事实均来自对当前代码(非文档)的直接读取,读取范围:`src/execution_log.py`、`src/work_graph_orchestrate.py`、`src/work_node_run.py`、`src/backend_server.py`(含其测试 `src/test_backend_server.py`/`src/test_work_graph_orchestrate.py`)、`docs/sdd/06-验收与测试锚点.md` REQ-13。 + +# BRD:打通执行日志真实写入路径,修复"暂无日志内容"与"下载崩溃"两个同根因 Bug + +## 0. 文档信息 + +- 关联需求锚点:`docs/sdd/06-验收与测试锚点.md` REQ-13("原始日志控制台内查看 + 定时清理",架构文档 13.1 节决策 4) +- 覆盖实体/接口(沿用 REQ-13 覆盖清单):M=`execution_log_file`;V=`v_log_viewer`;C=`c_execution_log_get` +- 类型:Bug 修复(单一根因,两个可观察症状),非新特性 +- 风险等级:低-中(不涉及权限/支付/数据不可逆变更;但触碰 `run_work_node`/`run_graph` 的核心执行闭环签名,属于"多处调用点的函数签名变更",需要回归验证) +- 状态机备注:本文档为 `task` 生命周期中 `planning_brd_prd_sdd` 阶段产出的 BRD Artifact,不代表任务已完成;状态流转由 `c_org_node_trigger`/`c_org_loop_route` 依据 BRD/PRD/SDD/Benchmark 判定 + +## 1. 背景与问题陈述 + +### 1.1 用户可观察的症状(事实,来自现有代码与用户报告的交叉验证) + +- **症状 A1**:控制台 Graph 页面中,任意一个 Work Node 视图点击后,日志区域始终显示"暂无日志内容",无论该节点实际是否已经执行完成。 +- **症状 A2**:点击"下载完整日志"按钮报错、页面跳转异常。 + +### 1.2 根因(已通过直接读取生产代码确认,非猜测) + +两个症状是**同一个根因**的两种表现:**日志文件从被创建到被消费的整条链路里,"写入"这一环从未被任何生产代码调用过。** + +具体证据: + +1. `src/execution_log.py:46` 的 `append_log(conn, node_exec_id, content)` 实现完整——写磁盘文件、更新 `execution_log_file.size_bytes`、写 `execution_event(event_type='log_appended')` 供 WS 实时订阅——但该模块自己的文件头注释(`src/execution_log.py:5-10`)已经明确写着:"`append_log()` is that missing write path, meant to be called from inside a run_fn as it works",即设计时就知道这是留给调用方接入的挂钩点,从未真正接入。 +2. 全仓库 `src/*.py` 生产代码中搜索 `append_log(` 调用,**零命中**(仅 `execution_log.py` 自身定义处、以及该模块自己的单元测试直接调用)。 +3. `src/work_node_run.py:262-268` 确实会在每个节点开始执行时 `INSERT INTO execution_log_file (...) VALUES (..., 'active')`,并在 `finally` 块(`src/work_node_run.py:410-414`)里把状态更新为 `'closed'`——即 `execution_log_file` 行的 **生命周期状态** 被正确管理,但这条记录指向的磁盘文件 `file_path` 自始至终从未被 `open(..., "a")` 写入过一个字节。 +4. `src/work_graph_orchestrate.py:141-155` 的 `_make_run_fn(target_folder_path, node_exec_id, org_node, prompt, output_schema, timeout)` 是真实节点执行的唯一入口(`run_fn` 闭包内部调用 `work_graph_execute.execute_work_graph(...)` 拿到 agent 的真实输出),但其函数签名里**没有 `conn` 参数**,闭包体内也没有任何日志写入调用。`run_graph()`(`work_graph_orchestrate.py:158`)本身持有 `conn`(是它的第一个形参),但从未把它继续传给 `_make_run_fn`。 +5. 因此:`src/execution_log.py:78` 的 `get_log()`(`GET /api/work-nodes/:node_exec_id/log` 的静态读取路径)在 `os.path.isfile(file_path)` 为 `False` 时会正确返回 `{"content": "", ...}`(`execution_log.py:90-91`)——前端把这个空 `content` 渲染成"暂无日志内容"是对空数据的**正确反应**,不是前端 bug,无需改动 `frontend/app.js`。 +6. `src/backend_server.py:823-831` 的 `_send_file_download(file_path, node_exec_id)` 直接 `open(file_path, "rb")`(第 824 行),**没有 `try/except FileNotFoundError`**。因为上述第 3 点,`file_path` 指向的文件从未被创建,`open()` 必然抛出未捕获的 `FileNotFoundError`,导致该请求以裸异常/500(或连接中断)方式失败,前端表现为"报错、跳转到不存在的页面"。对照同一文件里 `_handle_execution_log_get`(`backend_server.py:802-821`)已经为 `execution_log.LogNotFoundError`/`execution_log.LogPurgedError` 建立了"捕获特定异常 → 返回结构化 JSON 错误体"的风格(404/410),`_send_file_download` 是这条风格链里唯一的缺口。 + +### 1.3 为什么这是"同一个根因"而不是两个独立 bug + +A2 的 `FileNotFoundError` 之所以会被触发,前提正是 A1 描述的"日志文件从未被创建"。如果只修 A2(加 try/except),下载接口会变成"优雅地返回 404/空文件",但日志区域依然永远空白——用户的核心诉求(看到真实执行日志)完全没有被满足。反过来,如果只修 A1(接入 `append_log`),A2 的未捕获异常会在文件的边界场景(例如节点被判定为立即 `blocked`、从未真正跑到写日志那一步;或历史遗留、修复前创建的 `execution_log_file` 行)下依然存在。**两处都要修,且顺序上 A1 是使 A2 变得"通常不会触发"的必要条件,A2 是无论如何都该有的防御性错误处理(边界/异常场景永远存在)。** + +## 2. 目标与非目标 + +### 2.1 目标(Goals,均为用户可观察结果) + +- **G1**:任意一次真实 Work Node 执行完成后,控制台 Graph 页面点击该节点,日志区域能看到真实的、与该节点实际执行相关的日志内容(而不是"暂无日志内容")。 +- **G2**:点击"下载完整日志",无论该节点日志文件是否存在内容,都能得到一个明确、干净的响应(有内容则下载文件;文件确实不存在则返回结构化错误,而不是裸异常/崩溃跳转)。 +- **G3**:修复必须用**真实数据**验证,而不是仅靠 mock 单元测试通过——即真实跑一次 Work Graph 节点执行,确认磁盘上出现了非空日志文件、`GET /api/work-nodes/:node_exec_id/log` 读到非空 `content`、下载接口可用。 + +### 2.2 非目标(Non-Goals,本次不做,明确排除以防范围蔓延) + +- **不做**逐字符/逐 token 的真实流式日志(用户已在原始需求中明确说明:动态工作流本身是一次阻塞调用,`execute_work_graph()` 内部没有中间进度可取,"具体怎么划分日志条目由 PL/Coder 判断",只需要在真实调用前后各记一条)。 +- **不改动** `frontend/app.js` 的日志显示逻辑本身——已确认是对空数据的正确反应,问题在数据源侧,不在展示侧。 +- **不新增** `p_log_retention_cleanup` 调度器守护进程(`execution_log.py` 文件头注释已明确这是独立、未来的 Unit 范围,与本次两个 bug 无关,不得借此机会顺带实现)。 +- **不重新设计** `execution_log_file` 的表结构或状态机(`active`/`closed`/`purged` 三态生命周期管理已经正确,`src/work_node_run.py` 部分不需要改动)。 +- **不处理** `_verify_and_maybe_regenerate`(reflect 重试路径)之外的其他潜在日志缺口,除非它们复用同一个 `_make_run_fn`(见 3.3 节说明,reflect 路径也会自动受益,因为它复用同一个函数)。 + +## 3. 事实 / 设计决策 / 待验证假设 / 阻塞性未知 + +### 3.1 已知事实(有明确代码来源,见第 1 节脚注式引用) + +- `append_log()` 实现完整且从未被生产代码调用(`execution_log.py` + 全仓库 grep 结果)。 +- `_make_run_fn` 当前签名不含 `conn`,`run_fn` 闭包内不含任何日志写入(`work_graph_orchestrate.py:141-155`)。 +- `run_graph()` 自身持有 `conn`(第一形参),`_make_run_fn` 在 `run_graph()` 内被调用两处:节点首次执行(`work_graph_orchestrate.py:218`)与 reflect 重试路径(`work_graph_orchestrate.py:256`)。 +- `_send_file_download()` 无 `FileNotFoundError` 捕获(`backend_server.py:823-831`),同文件内已有 `LogNotFoundError`/`LogPurgedError` 的 404/410 JSON 错误响应先例可以直接复用其风格(`backend_server.py:816-819`)。 +- `execution_log_file` 的行生命周期(`active`→`closed`)由 `work_node_run.py` 正确管理,不需要改动。 +- REQ-13 的 happy-case 验收原文(`docs/sdd/06-验收与测试锚点.md:113`)直接写着"某 Work Node 运行中,`v_log_viewer` 能实时滚动显示新追加的日志内容"——这条验收锚点此前从未被真正满足过,本次修复是它第一次具备被满足的前提条件。 + +### 3.2 设计决策(PL 基于约束做出的选择,附理由) + +- **决策 D1**:`append_log()` 的调用点放在 `_make_run_fn()` 内部的 `run_fn` 闭包里,而不是放在更上层的 `run_graph()` 循环体里。 + 理由:`run_fn(cwd, model)` 是唯一真正拿到 agent 真实输出(`work_graph_execute.execute_work_graph(...)` 的返回值)的地方;`run_graph()` 循环体只拿到 `run_work_node()` 的最终结果(schema 校验后、含重试逻辑),如果在那一层记日志,会丢失"这次具体是第几次 attempt、真实原始输出是什么"的粒度,也无法在 agent 调用真正开始前就记一条"开始执行"日志(`run_work_node` 内部还有 worktree 创建、schema 校验等前置步骤,用户原始需求点名的是"真实调用前后")。这与用户需求原文的接入点选择一致。 +- **决策 D2**:`conn` 通过参数一路显式传递(`run_graph()` → `_make_run_fn()` → 闭包内部使用),不使用全局/线程局部变量或模块级单例。 + 理由:与本仓库既有约定一致——`run_work_node(conn, ...)`、`run_graph(conn, ...)` 均已是显式传参风格;`backend_server.py` 用 `self.server.conn_lock` + `self.server.conn` 管理并发访问,说明 `conn` 的访问需要受控(加锁),显式传参能保持调用链路可读、可测试(mock 一个 `conn` 即可单测,不需要打补丁全局状态)。 +- **决策 D3**:日志条目的具体切分("节点开始执行"一条 + "拿到真实输出"一条,或更细)由实现者判断,不在 BRD 里锁死成唯一方案。 + 理由:用户原始需求已明确"具体怎么划分日志条目由你判断,不需要做成逐字符真流式"——BRD 层面只约束**必须发生写入、且发生在真实调用前后**这个业务结果,写入粒度是实现细节,锁死会制造不必要的约束(违反"最小、可验证"的拆分原则)。 +- **决策 D4**:`_send_file_download()` 捕获 `FileNotFoundError` 后返回 404 + 结构化 JSON body(例如 `{"error": "LOG_FILE_NOT_FOUND"}` 或复用已有的 `LOG_NOT_FOUND` 语义,由实现者对齐同文件既有错误码命名风格),而不是让请求方对着一个二进制下载响应硬解析错误。 + 理由:与 `_handle_execution_log_get` 里已有的 404 `LOG_NOT_FOUND` / 410 `LOG_PURGED` 错误处理风格保持一致(`backend_server.py:816-819`),复用而不是新造一套错误处理约定。 + +### 3.3 待验证假设(需要在实现/验证阶段通过实际运行确认,不能只凭读代码判断) + +- **H1**:把 `conn` 加入 `_make_run_fn` 的签名后,两处调用点(`work_graph_orchestrate.py:218` 首次执行、`:256` reflect 重试)只需同步传参即可,不需要额外改动调用方逻辑——需要在实现后跑 `src/test_work_graph_orchestrate.py` 全量回归确认没有遗漏的第三个隐藏调用点或 mock 未同步更新的测试。 +- **H2**:`append_log()` 对同一个 `node_exec_id` 被多次调用(一次"开始"、一次"结束")时,`execution_log_file.size_bytes` 的递增更新和 `execution_event(log_appended)` 的多条写入不会与 `work_node_run.py` 的 `finally` 块(把 `execution_log_file.status` 置为 `'closed'`)产生时序冲突——`append_log()` 本身不检查 `status`,理论上不会冲突,但需要真实跑一次含 reflect 重试的节点(多次 append)来确认。 +- **H3**:`_send_file_download()` 补上异常捕获后,`Content-Length`/`Content-Disposition` 等响应头的写入时机(`send_response`/`send_header`/`end_headers` 调用顺序,`backend_server.py:826-830`)在改成先 `try: open()` 之后依然保持"要么完整发送 200+文件,要么在还没调用 `send_response` 之前就返回 404"的干净二选一,不会出现"已经发了部分响应头又要发错误 JSON"的畸形响应——需要实现时注意 `open()` 失败必须发生在任何 `self.send_response(...)` 调用之前。 + +### 3.4 阻塞性未知 + +无。本次两个 bug 的根因、接入点、错误处理风格均已通过读代码确认,不存在需要产品/业务/外部决策才能推进的未知项,可以直接进入开发。 + +## 4. 范围(Scope) + +### 4.1 范围内文件 + +- `src/execution_log.py`:**不修改**其内部实现(`append_log()` 已经完整、正确),仅作为被调用方。 +- `src/work_graph_orchestrate.py`:`_make_run_fn()` 增加 `conn` 参数;`run_fn` 闭包内,在调用 `work_graph_execute.execute_work_graph(...)` 前后各调用一次 `execution_log.append_log(conn, node_exec_id, ...)`;`run_graph()` 内两处 `_make_run_fn(...)` 调用点同步传入 `conn`(`run_graph()` 自身已持有该参数,无需改动其自身签名)。 +- `src/backend_server.py`:`_send_file_download()` 增加 `FileNotFoundError` 捕获,返回结构化 404 JSON 错误体;`_handle_execution_log_get()` 中调用 `_send_file_download` 的位置视实现需要可能需要同步调整异常捕获层级(由实现者判断放在 `_send_file_download` 内部捕获更贴近现有 `LogNotFoundError`/`LogPurgedError` 的捕获风格所在层级,还是在 `_send_file_download` 内部自行 try/except 并返回)。 +- 对应测试文件:`src/test_work_graph_orchestrate.py`、`src/test_backend_server.py`(mock 层面的单元测试更新,验证签名变更和错误处理,覆盖真实数据验证之外的边界/异常场景)。 + +### 4.2 明确排除 + +- `frontend/app.js`(已确认无需改动,见 2.2)。 +- `src/execution_log.py` 内部实现、`src/work_node_run.py` 的 `execution_log_file` 状态机管理。 +- 调度器/`p_log_retention_cleanup` 相关代码。 + +## 5. 验收标准(Definition of Done) + +必须**同时满足**以下三类证据,缺一不可(用户已明确要求"修完后请用真实数据验证",因此本次 Done 标准比常规更严格,Mock 单测通过不足以视为完成): + +### 5.1 单元测试证据(mock 边界,回归现有行为) + +- `src/test_work_graph_orchestrate.py`:新增/更新用例断言 `run_fn` 执行路径中,`execution_log.append_log`(或对其效果的可观察断言,例如断言 mock conn 上执行了预期的 SQL/断言磁盘上真实生成的日志文件内容)在真实 agent 调用前、后各被触发一次;已有全部用例(含 reflect 重试路径 `_verify_and_maybe_regenerate` 相关用例)在 `_make_run_fn` 签名变更后依然全部通过。 +- `src/test_backend_server.py`:新增用例覆盖"`execution_log_file` 行存在但磁盘文件已被外部删除/从未创建"场景下 `download=true` 请求,断言返回结构化 404 JSON(而非抛出未捕获异常/连接中断);已有 `test_download_returns_raw_file_with_attachment_headers`(`src/test_backend_server.py:1135`)等既有下载用例保持通过。 +- 命令:`python3 -m unittest discover -s src`(本仓库唯一的后端测试根,见 `CLAUDE.md`)全量通过,不允许因本次改动引入任何新的失败或被跳过的用例。 + +### 5.2 真实数据验证证据(用户明确要求,不可用 mock 替代) + +- 真实跑一次任意 Work Graph 节点执行(例如通过 `e2e_pipeline.py` 走一遍真实闭环,或针对 `run_graph()` 单独构造一次真实调用),执行完成后: + 1. 该节点对应的 `execution_log_file.file_path` 磁盘文件**真实存在且非空**(`os.path.getsize(...) > 0`)。 + 2. `GET /api/work-nodes/:node_exec_id/log`(`execution_log.get_log()` 路径)返回的 `content` 字段非空,且内容与该节点真实执行的信息相关(不是占位符/固定字符串)。 + 3. 下载接口(`download=true` 路径)能正常返回该文件的完整二进制内容,响应头 `Content-Disposition`/`Content-Length` 与文件实际大小一致,不报错、不跳转异常页面。 +- 这一步的执行记录(命令、输出摘要)应作为验证证据保留,供 Validator 复核(对应 SOUL.md §9 可追溯原则:"原始要求 → 主任务目标/验收 → 责任 Unit → 测试证据")。 + +### 5.3 边界/异常场景证据 + +- `execution_log_file` 行存在、但因为节点在 `append_log` 被调用之前就失败(例如 `InvalidInputError`,`work_node_run.py:226-227` 那种"校验失败直接拒绝,不进入执行"的场景)导致文件从未被创建的情况下: + - `GET .../log`(非下载)应继续返回 `{"content": "", ...}`(`execution_log.py` 现有行为,不应回归)。 + - `GET .../log?download=true` 应返回结构化 404 JSON,而不是裸异常(这正是 A2 的核心修复目标,必须显式测试,不能只靠"日志现在总是有内容了所以问题自然消失"这种侥幸覆盖)。 +- 已 `purged`(被清理)的日志:`LogPurgedError` 路径(410)不应被本次改动破坏。 + +## 6. 风险与依赖 + +- **风险 R1(签名变更影响面)**:`_make_run_fn` 增加 `conn` 参数是一个函数签名变更,影响 `run_graph()` 内两个调用点和所有依赖 `work_graph_orchestrate` 模块的测试/调用方(`src/e2e_pipeline.py:226,269` 通过 `run_graph()` 间接调用,`e2e_pipeline.py` 本身不直接调用 `_make_run_fn`,风险较低,但仍需回归 `src/test_work_graph_orchestrate.py` 全量用例确认)。 +- **风险 R2(并发写入)**:`append_log()` 内部会执行 `conn.execute` + `conn.commit()`;`backend_server.py` 对 `self.server.conn` 的访问统一走 `self.server.conn_lock`(如 `backend_server.py:807,814`),但 `run_graph()`/`work_node_run.py` 侧目前直接使用传入的 `conn` 而不显式加这把锁——如果 Work Graph 执行与 HTTP 请求处理共享同一个 `conn`/同一个 SQLite 连接且跨线程,需要确认现有加锁机制是否已经覆盖了这条新增的写入路径,避免引入 SQLite "database is locked"类问题。这是实现阶段需要确认的风险点,不属于阻塞性未知(现有代码库已有 `conn_lock` 先例可以参照,不是从零设计)。 +- **依赖**:本次改动不依赖任何未完成的其他 Unit;`execution_log.py`、`work_node_run.py` 均已是稳定、可直接复用的现成实现。 + +## 7. 可追溯性矩阵 + +| 原始要求 | 责任范围 | 验收标准锚点 | +|---|---|---| +| 修复 A1:日志区域显示"暂无日志内容" | `work_graph_orchestrate.py` 接入 `append_log` | 5.1(单测)+ 5.2.1/5.2.2(真实数据) | +| 修复 A2:下载报错/跳转异常页面 | `backend_server.py` `_send_file_download` 异常处理 | 5.1(单测)+ 5.2.3(真实数据)+ 5.3(边界场景) | +| "conn 一路传进 `_make_run_fn()`" | `_make_run_fn`/`run_graph` 签名变更 | 5.1 全量回归通过(R1) | +| "真实调用前后各记一条日志,粒度自行判断" | `run_fn` 闭包内部实现 | 3.2 D3 + 5.2.2(内容非占位符) | +| "不需要逐字符真流式" | 非目标声明 | 2.2 | +| "不改动 frontend/app.js" | 非目标声明 | 2.2、1.2 第 5 点事实依据 | +| "用真实数据验证" | 验证阶段 | 5.2 全部三项 | +| REQ-13 happy-case(`v_log_viewer` 能显示追加日志) | 整体修复 | 5.2 | + +## 8. PL 自审(Failure-Closed Checklist) + +- [x] 根因是否有明确代码来源,而非猜测:是(第 1.2 节六条证据均标注了具体文件行号)。 +- [x] 是否区分了事实/决策/假设/未知:是(第 3 节四类分开列出)。 +- [x] 是否存在阻塞性未知:否(3.4 节明确结论)。 +- [x] 范围是否清晰、是否显式排除了容易蔓延的相邻工作(流式日志、调度器、前端改动):是(第 2.2、4.2 节)。 +- [x] 验收标准是否可测试、是否描述可观察结果而非内部实现细节:是(第 5 节区分单测/真实数据/边界三类,且真实数据验证是用户显式要求、不可被 mock 替代的硬性条件)。 +- [x] 依赖是否显式、是否存在环:无依赖、无环(第 6 节)。 +- [x] 原始要求是否都能在追溯矩阵中找到责任方和验收锚点:是(第 7 节)。 +- [x] 是否需要更深调查(权限/支付/不可逆数据变更等高风险场景):否,本次改动风险等级为低-中,已识别的风险点(R1 签名影响面、R2 并发加锁)都有现成模式可参照,不需要额外调查即可进入开发;建议 Code Agent 在改动 `_send_file_download` 时优先复用 `_handle_execution_log_get` 已有的错误响应风格,不新造一套错误码命名。 + +本 BRD 判定:**可以流转到开发(Code Agent)**,无阻塞性未知,范围与验收标准明确。后续 PRD/SDD(如该流水线要求)应在此基础上补充 `_make_run_fn`/`_send_file_download` 的具体函数级签名设计,BRD 本身不预先锁定实现代码。 -- libgit2 0.21.4