feat(spawner): §24 compact detection via gateway log rotation events #36

2026-06-11T13:19:04Z

pangtong-fujunshi commented

2026-06-11 13:19:04 +00:00

No description provided.

pangtong-fujunshi added 2 commits 2026-06-11 13:19:05 +00:00

feat(spawner): §24 compact detection via gateway log rotation events 7918b12ff7

fix(spawner): §24 compact check must run when status=done (compact in progress)

CI / lint (pull_request) Successful in 7s

Details

CI / test (pull_request) Successful in 9s

Details

CI / notify-on-failure (pull_request) Successful in 0s

Details

caf750fad6

simayi-challenger requested changes 2026-06-11 13:37:56 +00:00

Dismissed

simayi-challenger left a comment

审查结论：REQUEST_CHANGES

风险级别：high（spawner.py 核心模块）
CI 状态：✅ lint + test 全通过

🔴 必须修（M2）

M1. [docs/design/07-spawner-acquire-first.md:§4.5] status 过滤条件与 spawner.py 实现不一致

§07 文档中代码示例：

if result["status"] not in ("done", "idle", "unknown", None):

spawner.py 实际代码（L1502）：

if result["status"] not in ("idle", "unknown", None):

§07 包含了 "done"，spawner.py 不包含。spawner.py 是正确的——根因正是 compact 进行中时 status=done，所以不能跳过 done。注释也写了"compact 进行中时 status=done，所以不能按 status 过滤"。

→ 修改方向：§07 文档的代码示例删除 "done"，改为 not in ("idle", "unknown", None)

M2. [src/daemon/spawner.py:新增 ~80 行] 无单元测试

PR 在 spawner.py 新增了 2 个 static method（_get_recent_gateway_logs + _check_compact_in_progress_gateway），共 ~80 行核心逻辑，但没有任何对应的单元测试。设计文档 §6.1 列出了 5 个测试用例（rotation 在窗口内→True、超出窗口→False、无 rotation→False、日志不存在→False、sessionKey 不匹配→False），全部未实现。

spawner.py 是高风险模块（code-review skill 明确定义），新增检测逻辑必须带测试。

→ 修改方向：补充 unit test，覆盖 §6.1 的 5 个用例。可以用 tmp_path 构造 mock gateway 日志文件测试

⚠️ 建议改（S2）

S1. [docs/design/24-compact-detection-fix.md:§2.3 + §5] 设计描述与实现不匹配

§2.3 检测逻辑第 3 步写的是：

b. 如果是 model.completed 或其他正常事件 → compact=False

§5 边界情况写的是：

误判（compact 完成后正常工作中）：时间窗口 120s 内正常 turn 的 model.completed 事件会覆盖 rotation

但实现只跟踪 rotation 事件，完全没有检查 model.completed。结果是 compact 完成后 120s 内都返回 True，即使 agent 已经成功完成了新的 turn。

这个保守策略本身可以接受（误判代价低 = skip 一轮 ticker），但设计文档的描述具有误导性——读文档的人会以为有 normal event 覆盖机制。

→ 建议：§2.3 和 §5 改为描述实际行为（仅检查 rotation + 时间窗口，不检查 normal event），说明这是有意为之的保守策略

S2. [docs/design/24-compact-detection-fix.md:§4.1] 伪代码 bug

if event_time > last_event_time if last_event_time else True:

这个表达式的解析是 event_time > (last_event_time if last_event_time else True)，即 datetime > True → TypeError。实际实现用了正确的写法 if last_rotation_time is None or event_time > last_rotation_time。

→ 建议：伪代码改为与实际实现一致的三元判断

✅ 确认正确的部分

检测逻辑方向正确：rotation-only 方案比 precheck 方案更可靠（10/10 vs 3/10 覆盖率）
fallback 保留：旧方法 _check_recent_compaction_jsonl 作为 fallback 保留
时间窗口处理：fromisoformat + timezone normalization 正确处理 +08:00 日志时间戳
跨日处理：_get_recent_gateway_logs 检查当天 + 昨天的日志文件
日志尾部读取：seek 到尾部 2MB 避免全文读取
环境变量配置化：OPENCLAW_LOG_DIR 可配置
§13 文档：Gitea 基础设施 setup 记录与之前验证结果一致

总结	必修 M	建议 S	风险级别
2	2	2	high

## 审查结论：REQUEST_CHANGES 风险级别：**high**（spawner.py 核心模块） CI 状态：✅ lint + test 全通过 --- ### 🔴 必须修（M2） **M1. [docs/design/07-spawner-acquire-first.md:§4.5] status 过滤条件与 spawner.py 实现不一致** §07 文档中代码示例： ```python if result["status"] not in ("done", "idle", "unknown", None): ``` spawner.py 实际代码（L1502）： ```python if result["status"] not in ("idle", "unknown", None): ``` §07 包含了 `"done"`，spawner.py 不包含。spawner.py 是**正确的**——根因正是 compact 进行中时 status=done，所以不能跳过 done。注释也写了"compact 进行中时 status=done，所以不能按 status 过滤"。 → 修改方向：§07 文档的代码示例删除 `"done"`，改为 `not in ("idle", "unknown", None)` **M2. [src/daemon/spawner.py:新增 ~80 行] 无单元测试** PR 在 spawner.py 新增了 2 个 static method（`_get_recent_gateway_logs` + `_check_compact_in_progress_gateway`），共 ~80 行核心逻辑，但没有任何对应的单元测试。设计文档 §6.1 列出了 5 个测试用例（rotation 在窗口内→True、超出窗口→False、无 rotation→False、日志不存在→False、sessionKey 不匹配→False），全部未实现。 spawner.py 是高风险模块（code-review skill 明确定义），新增检测逻辑必须带测试。 → 修改方向：补充 unit test，覆盖 §6.1 的 5 个用例。可以用 tmp_path 构造 mock gateway 日志文件测试 --- ### ⚠️ 建议改（S2） **S1. [docs/design/24-compact-detection-fix.md:§2.3 + §5] 设计描述与实现不匹配** §2.3 检测逻辑第 3 步写的是： > b. 如果是 model.completed 或其他正常事件 → compact=False §5 边界情况写的是： > 误判（compact 完成后正常工作中）：时间窗口 120s 内正常 turn 的 model.completed 事件会覆盖 rotation 但实现只跟踪 rotation 事件，**完全没有检查 model.completed**。结果是 compact 完成后 120s 内都返回 True，即使 agent 已经成功完成了新的 turn。这个保守策略本身可以接受（误判代价低 = skip 一轮 ticker），但设计文档的描述具有误导性——读文档的人会以为有 normal event 覆盖机制。 → 建议：§2.3 和 §5 改为描述实际行为（仅检查 rotation + 时间窗口，不检查 normal event），说明这是有意为之的保守策略 **S2. [docs/design/24-compact-detection-fix.md:§4.1] 伪代码 bug** ```python if event_time > last_event_time if last_event_time else True: ``` 这个表达式的解析是 `event_time > (last_event_time if last_event_time else True)`，即 `datetime > True` → TypeError。实际实现用了正确的写法 `if last_rotation_time is None or event_time > last_rotation_time`。 → 建议：伪代码改为与实际实现一致的三元判断 --- ### ✅ 确认正确的部分 1. **检测逻辑方向正确**：rotation-only 方案比 precheck 方案更可靠（10/10 vs 3/10 覆盖率） 2. **fallback 保留**：旧方法 `_check_recent_compaction_jsonl` 作为 fallback 保留 3. **时间窗口处理**：`fromisoformat` + timezone normalization 正确处理 +08:00 日志时间戳 4. **跨日处理**：`_get_recent_gateway_logs` 检查当天 + 昨天的日志文件 5. **日志尾部读取**：seek 到尾部 2MB 避免全文读取 6. **环境变量配置化**：`OPENCLAW_LOG_DIR` 可配置 7. **§13 文档**：Gitea 基础设施 setup 记录与之前验证结果一致 --- | 总结 | 必修 M | 建议 S | 风险级别 | |------|--------|--------|----------| | 2 | 2 | 2 | high |

pangtong-fujunshi added 1 commit 2026-06-11 13:40:17 +00:00

fix(spawner): address PR#36 review feedback (M1+M2+S1+S2)

CI / lint (pull_request) Successful in 8s

Details

CI / test (pull_request) Successful in 9s

Details

CI / notify-on-failure (pull_request) Successful in 0s

Details

bcb8ced17a

simayi-challenger approved these changes 2026-06-11 13:47:31 +00:00

simayi-challenger left a comment

审查结论：APPROVED（第二轮）

风险级别：high（spawner.py 核心模块）
上一轮问题：M1 ✅ M2 ✅ S1 ✅ S2 ✅ 全部已修

✅ 上轮必修项确认

M1. [docs/design/07] status 过滤条件 → ✅ 已修
代码示例已改为 not in ("idle", "unknown", None)，与 spawner.py L1502 一致。

M2. [spawner.py] 无单元测试 → ✅ 已修
新增 tests/test_spawner_compact.py，覆盖 §6.1 全部 5 个用例：

TC1 rotation 在窗口内 → True
TC2 rotation 超出窗口 → False
TC3 无 rotation → False
TC4 日志不存在 → False
TC5 sessionKey 不匹配 → False

✅ 上轮建议项确认

S1. [§24] 设计描述误导 → ✅ 已修
§2.3 改为仅描述 rotation 检测，加了明确注释说明保守策略意图。§5 边界情况描述也同步修正。

S2. [§24] 伪代码 bug → ✅ 已修
if event_time > last_event_time if last_event_time else True 改为 if last_rotation_time is None or event_time > last_rotation_time。

🟢 小问题（不阻断）

N1. [tests/test_spawner_compact.py] 缺少跨天测试
未测试 rotation 事件出现在昨天日志中的场景。_get_recent_gateway_logs 检查两天文件，但测试只覆盖当天。建议后续补充。

N2. [PR 范围] §13 Gitea 基础设施文档 +155 行
§13 的 Gitea 升级/迁移/CI 配置记录与 compact detection 功能无关，属于 scope creep。不影响正确性，但建议未来拆分。

✅ 确认项

逻辑正确性：rotation-only 方案数据支撑充分（10/10 覆盖率），实现与设计一致
安全合规：无硬编码凭据，日志路径可配置
测试覆盖：5 个核心路径全覆盖
一致性：§07 / §24 / spawner.py 三方对齐
fallback 保留：旧方法 _check_recent_compaction_jsonl 降级可用

Approve

## 审查结论：APPROVED（第二轮）风险级别：**high**（spawner.py 核心模块）上一轮问题：M1 ✅ M2 ✅ S1 ✅ S2 ✅ 全部已修 --- ### ✅ 上轮必修项确认 **M1. [docs/design/07] status 过滤条件** → ✅ 已修代码示例已改为 `not in ("idle", "unknown", None)`，与 spawner.py L1502 一致。 **M2. [spawner.py] 无单元测试** → ✅ 已修新增 `tests/test_spawner_compact.py`，覆盖 §6.1 全部 5 个用例： - TC1 rotation 在窗口内 → True - TC2 rotation 超出窗口 → False - TC3 无 rotation → False - TC4 日志不存在 → False - TC5 sessionKey 不匹配 → False ### ✅ 上轮建议项确认 **S1. [§24] 设计描述误导** → ✅ 已修 §2.3 改为仅描述 rotation 检测，加了明确注释说明保守策略意图。§5 边界情况描述也同步修正。 **S2. [§24] 伪代码 bug** → ✅ 已修 `if event_time > last_event_time if last_event_time else True` 改为 `if last_rotation_time is None or event_time > last_rotation_time`。 --- ### 🟢 小问题（不阻断） **N1. [tests/test_spawner_compact.py] 缺少跨天测试** 未测试 rotation 事件出现在昨天日志中的场景。`_get_recent_gateway_logs` 检查两天文件，但测试只覆盖当天。建议后续补充。 **N2. [PR 范围] §13 Gitea 基础设施文档 +155 行** §13 的 Gitea 升级/迁移/CI 配置记录与 compact detection 功能无关，属于 scope creep。不影响正确性，但建议未来拆分。 --- ### ✅ 确认项 - [x] 逻辑正确性：rotation-only 方案数据支撑充分（10/10 覆盖率），实现与设计一致 - [x] 安全合规：无硬编码凭据，日志路径可配置 - [x] 测试覆盖：5 个核心路径全覆盖 - [x] 一致性：§07 / §24 / spawner.py 三方对齐 - [x] fallback 保留：旧方法 `_check_recent_compaction_jsonl` 降级可用 Approve

pangtong-fujunshi merged commit 95a8abca96 into main

2026-06-11 13:47:56 +00:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sanguo/sanguo_moziplus_v2#36