diff --git a/.claude/agent-memory/planner/MEMORY.md b/.claude/agent-memory/planner/MEMORY.md index cc04dc3..d5217e2 100644 --- a/.claude/agent-memory/planner/MEMORY.md +++ b/.claude/agent-memory/planner/MEMORY.md @@ -9,3 +9,4 @@ - [deploy_stage.py ConcurrentDeploymentError PRD: consistent](project_deploy_stage_concurrent_conflict_prd.md) — reviewed 2026-08-07, no contradictions; all facts (staged→candidate_starting transition, ux_deployment_active terminal set) verified against code - [skills detail-content bug: already fixed](project_skills_detail_content_bug_already_fixed.md) — get_skill_content/route/panel/tests all present 2026-08-13; only unverified is git commit/Validator status, not missing code - [judge_prompt must reject narrative-wrapper output](feedback_judge_prompt_narrative_wrapper.md) — brd/sdd/benchmark judge_prompts need explicit "no completion-status framing, no hedged-deferred content, no prose summarizing what should be a table" instruction, generic quality prompts don't catch it +- [agent-call-liveness BRD written 2026-08-13](project_agent_call_liveness_brd_written.md) — full call-site inventory + scheduler blocked_reason registry mechanism to reuse; core signal-selection question deliberately left open for SDD diff --git a/.claude/agent-memory/planner/project_agent_call_liveness_brd_written.md b/.claude/agent-memory/planner/project_agent_call_liveness_brd_written.md new file mode 100644 index 0000000..5e6ac5e --- /dev/null +++ b/.claude/agent-memory/planner/project_agent_call_liveness_brd_written.md @@ -0,0 +1,44 @@ +--- +name: project-agent-call-liveness-brd-written +description: BRD for "agent call timeout must be liveness-based, not fixed-duration" written 2026-08-13; call-site inventory and open questions for SDD stage +metadata: + type: project +--- + +BRD written 2026-08-13 at `docs/todo/agent-call-liveness-not-fixed-duration-timeout-brd.md`, +based on requirements doc `docs/todo/agent-call-liveness-not-fixed-duration-timeout-requirements.md`. +Triggered by a real incident: task `bfd3ee82`'s Validator planning call (large embedded SDD/PRD +prompt) hit the 300s fixed timeout in `trigger_org_node_structured` and was misclassified as +`blocked_reason='execution_error'` via scheduler.py's catch-all (not a real hang). + +**Why:** this closes the gap already flagged in +[[feedback_agent_call_liveness_not_fixed_timeout]] (recorded same day, before the BRD existed) — +the "no fixed-duration timeout" rule had been agreed before but never implemented; `bfd3ee82` is +the first real incident it caused, not the first time the rule was stated. + +**How to apply (facts verified 2026-08-13, re-check before trusting long-term):** +- Only `e2e_pipeline.py:1131` (`run_test_script_for_real`) catches `subprocess.TimeoutExpired` + specifically — everywhere else it falls through to the `execution_error` catch-all + (`task_execution.py::_mark_blocked` line 223, `scheduler.py` line 255-256). +- Full call-site inventory with fixed `timeout=` values (300/600/1200/180 etc, all fixed-duration + wall-clock): `org_node_trigger.py` (the two entry points), `e2e_pipeline.py` (plan/review/ + implement/validate, several call sites), `work_graph_plan.py::plan_work_graph`, + `work_graph_orchestrate.py` (judgment/reflection/skill-fix, several), `org_level_loop.py:: + run_fix_loop`, `root_cause_trace.py::real_validator_perspective`, `org_node_lifecycle.py:: + trigger_org_node_full`, `work_graph_execute.py::execute_work_graph` (separate — Dynamic + Workflow script exec, not a direct `claude -p` call, scoped as an open question not + pre-included). +- `scheduler.py::_BLOCKED_REASON_FOR_EXCEPTION` (line 82-89) is the existing exception-type → + blocked_reason registry to extend with a new "real stall" exception type → + `agent_call_stalled`, rather than inventing a parallel routing mechanism. +- `schema.py`'s `blocked_reason` column is unconstrained TEXT (no CHECK), and `frontend/app.js` + only escapes/displays the raw string generically (no hardcoded enum switch beyond + `user_paused`/`user_stopped`) — adding `agent_call_stalled` needs no schema migration and no + frontend enum sync, unlike the drift pattern in [[project_task_status_enum_drift_gap]]. +- BRD deliberately left the core technical question open for SDD: what "liveness signal" is even + observable, given `claude -p --output-format json` is one-shot, not streaming (needs real + research, not an assumption) — see BRD's 阻塞未知/open-questions section before writing PRD/SDD. +- Related but independent Goal: `docs/todo/boss-agent-autonomous-blocked-task-arbiter- + requirements.md` (handles what to do once a task IS truly stuck in blocked_pending_human) — + this BRD's fix should land first to reduce false-positive noise Boss Agent would otherwise have + to triage. diff --git a/docs/todo/agent-call-liveness-not-fixed-duration-timeout-brd.md b/docs/todo/agent-call-liveness-not-fixed-duration-timeout-brd.md new file mode 100644 index 0000000..08501c0 --- /dev/null +++ b/docs/todo/agent-call-liveness-not-fixed-duration-timeout-brd.md @@ -0,0 +1,258 @@ +# BRD：Agent 调用超时判定改为"存活性检测"，不再用固定时长 + +**日期**：2026-08-13 +**来源**：`docs/todo/agent-call-liveness-not-fixed-duration-timeout-requirements.md`（本 BRD 是对该 +需求文档的 Planner 结构化产出，两者应一起读；本文档不重复该文档已经讲清楚的背景细节，只在需要 +补充事实依据、结构化 WHY/HOW/WHAT、或做取舍判断的地方展开）。 +**性质**：BRD（组织级需求文档），不含详细技术设计——技术方案（存活信号选型、阈值、call site 改造 +顺序）留给 SDD 阶段，PL 在本文档中不预设答案。 +**状态**：待自审通过后进入 PRD/SDD 阶段。 + +--- + +## 0. 一句话目标 + +把 `src/org_node_trigger.py`（以及所有 `claude -p` 调用点）从"总耗时超过固定 N 秒就判定超时/卡死" +改为"距上一次可观察的活跃信号超过 N 秒才判定卡死"，从根子上消除"内容量大、真实需要更长时间的正常 +调用被误杀成 `blocked_pending_human`"这一类系统性误判。 + +--- + +## A. WHY + +### (a) 我们为什么要做这件事情？ + +这是一个**已经发生过、且会重复发生**的系统性缺陷，不是一次性个例： + +- 今天（2026-08-13）任务 `bfd3ee82` 的 Validator 规划调用（prompt 中嵌入完整已批准的 SDD/PRD + 全文，内容量大）跑满 `trigger_org_node_structured` 默认的 300 秒被 `subprocess.TimeoutExpired` + 杀掉，未被 `scheduler._BLOCKED_REASON_FOR_EXCEPTION` 任何一条已知类型捕获，落入 + `task_execution.py::_mark_blocked` 的兜底分支，被打成 `blocked_reason='execution_error'`， + 状态变为 `blocked_pending_human`——但这次调用本身并没有卡死，只是内容量大、真实耗时超过了 + 预先猜的固定数字（`src/task_execution.py:222-223`、`src/scheduler.py:255-256` 均已核实为 + 这个"未匹配类型 → execution_error"的兜底路径，`subprocess.TimeoutExpired` 目前只在 + `e2e_pipeline.py:1131` 一处被专门捕获，其余所有调用点都会走到这个兜底分支）。 +- 这不是新发现的认知——本次会话之前已经明确讨论并确认过"不能用固定总时长判断卡死"这个原则 + （见需求文档 header："用户明确重申过的既有决定"），本 Agent 自己的 memory 中也已记录 + `feedback_agent_call_liveness_not_fixed_timeout.md`："never judge a `claude -p` call 'stuck' + by fixed duration, only by no-output+no-error; org_node_trigger.py still violates this + (2026-08-13)"。**这意味着这不是一个未被意识到的缺陷，而是一个已经被明确否决、但从未被真正 + 修复过的实现**——今天的 `bfd3ee82` 是这个已知缺口第一次造成真实的任务被误判阻塞的记录在案的 + 实例。 +- 重复性风险：`trigger_org_node`/`trigger_org_node_structured` 是**所有** org_node（Planner、 + Planner-Reviewer、Coder、Validator）调用 `claude -p` 的唯一入口（`org_node_trigger.py` 模块 + docstring 自述），已核实的调用点覆盖 `e2e_pipeline.py`（规划/复审/实现/验证四类调用， + timeout 300/1200 不等）、`work_graph_plan.py::plan_work_graph`、`work_graph_orchestrate.py` + （判定/反思/skill 修复等多处 timeout=180/300）、`org_level_loop.py::run_fix_loop`、 + `root_cause_trace.py::real_validator_perspective`、`org_node_lifecycle.py::trigger_org_node_full` + ——只要 prompt 内容量、目标仓库规模、或任务复杂度比写死的 300/1200 秒假设的场景更大，就会 + 重复触发同一类误杀。目标仓库越大、SDD/PRD 越完整、Work Graph 越复杂，这个概率只会上升，不会 + 下降。 +- 回归成本：一次误杀的直接代价是任务被错误地打上 `blocked_pending_human` + `execution_error`， + 需要人工介入才能恢复（且 `execution_error` 是笼统兜底标签，人工排查时看不出"这只是跑得慢"还是 + "真的坏了"，需要重新翻日志才能分辨）——在 7x24 lights-out 的目标下，这类噪音会持续消耗人工 + 仲裁的信任度和精力，也会污染未来 Boss Agent（另一 Goal）要处理的"卡住任务"信号质量。 + +### (b) 它的目的是什么？ + +预期可观察结果： +- 一次 agent 调用只要还在真实产出/推进（不管是 stdout 增量、进程活动，还是其它可观察的"进行中" + 信号），无论总耗时多长（哪怕远超今天的 300/1200 秒），都不会被系统判定为超时。 +- 一次调用只有在**长时间没有任何新输出、且没有报错**（真正卡死、进程挂起不动）时，才会在有界的 + 时间内被判定为异常，并带上专属、可区分的原因标签（如 `agent_call_stalled`），而不是被淹没在 + 笼统的 `execution_error` 里。 + +### (c) 它为谁解决什么价值问题？ + +| 角色 | 痛点 | 可验证收益 | +|---|---|---| +| graphlemon 自身（自托管流水线） | 大 prompt/大仓库/复杂任务的正常调用被固定墙钟误杀，占用 `blocked_pending_human`，阻断自动流转 | 正常调用不再因超时被误杀，`blocked_pending_human` 中的真实占比下降 | +| 未来的 Boss Agent（`docs/todo/boss-agent-...` 需求文档） | 需要对 `blocked_pending_human` 任务做自主仲裁，如果一半以上是"假异常"（其实是慢调用），仲裁信号被严重稀释 | 拿到的 `blocked_pending_human` 信号更干净，`agent_call_stalled` 是一个专属、可信的"真卡死"分类，减少 Boss Agent 需要处理的噪音 | +| 人工介入者（今天仍需人工恢复 `blocked_pending_human` 的场景） | 看到 `execution_error` 无法判断是"跑太久"还是"真故障"，需要重新翻日志才能分辨 | `agent_call_stalled` vs 其它已分类原因一目了然，减少排查成本 | + +### (d) 与历史失效对齐吗？ + +对齐，且是"同类缺陷、规范已定但未落地执行"的典型样本，不是新问题： + +- 用户此前已经明确确立过这个原则（本文档开头即写明"既有决定"），本 Agent 自身 memory 也已经 + 记录了这条规则本身正在被违反（`feedback_agent_call_liveness_not_fixed_timeout.md`）——说明 + 仅仅"讨论过、确认过"不足以让实现跟上，必须落成一个可验证、可回归测试的真实改动，并在验收标准 + 里包含"复现 `bfd3ee82` 场景不再被误杀"这一具体回归项，而不能只停留在"以后注意"。 +- 与本仓库另一个已知模式吻合（`docs/development-plan.md` U-013 系列 incident notes、CLAUDE.md + "Two things named Supervisor"一节反复强调的"不能假设讨论过就等于已实现"）：文档/决定与代码 + 实际行为脱节是这个项目在自托管开发中反复出现过的真实事故类别，这次是同一类脱节在"调用超时判断" + 这个具体点上的又一次体现。 +- 与 `docs/done/validator-loop-until-dry-skill-requirements.md`"一次 `agent()` 调用会话自己 + 收敛需要多轮"是同一个认知内核（需求文档 §1 已指出）：单次调用内部就是一个多轮工具调用的会话， + 外部不该用一个预先猜的固定数字去卡这个内部收敛过程的时长。 + +### (e) 为什么是"基础设施级"修复，而不是单点修一次 `bfd3ee82`？ + +- `trigger_org_node`/`trigger_org_node_structured` 是唯一入口，前面列出的调用点分散在 + `e2e_pipeline.py`/`work_graph_plan.py`/`work_graph_orchestrate.py`/`org_level_loop.py`/ + `root_cause_trace.py`/`org_node_lifecycle.py` 六个模块里，各自传入不同的固定 timeout + （180/300/600/1200 秒），如果只针对 `bfd3ee82` 这一次 Validator 规划调用手动调大它的 + timeout 数值，既治标不治本（下一次更大的 prompt/更复杂的任务照样会撞到新的固定值），也不会 + 修正判断逻辑本身"用总时长而不是活跃度判断"的根本错误——需求文档 §2 已经把范围明确定义为 + "改任何调用 `claude -p` 的地方"，本 BRD 延续这个范围判断：这是一个要在唯一入口层面（或至少 + 在一个被所有调用点复用的公共判断函数层面）解决的基础设施问题，不是某个调用点的参数调优问题。 + 没有一个组织级/公共层面的修复，同一个 bug 会在每个新增调用点、每次目标仓库变大时重复发生。 + +--- + +## B. HOW（机制，不含实现细节） + +本次修复**不引入新的 Org/Work Graph 节点、边或并行拓扑**——它改动的是既有单一执行边界 +（`org_node_trigger.py` 的 `subprocess.run(..., timeout=timeout)` 调用）内部的判断逻辑， +不是设计一张新图。因此项目级 BRD 骨架中"Org/Work 双层分工、14-step 映射、fan-out/merge/barrier" +等条款在本次不强行套用（无分叉、无新节点，遵循"不分叉就别造图"的既有原则）；本节只说明这次修复 +落在既有哪些机制的边界内、复用哪些既有约定： + +- **触发层**（`src/org_node_trigger.py`）：所有真实 org_node 调用 `claude -p` 的唯一入口，本次 + 修复的判断逻辑主体就在这里（`trigger_org_node`/`trigger_org_node_structured` 各自的 + `subprocess.run(timeout=...)` 调用点）——不新增 Org 角色，不改变"identity 来自 + `.claude/agents/.md`"这条既有约定。 +- **另一个独立的固定超时点**（`work_graph_execute.py::execute_work_graph`，Dynamic Workflow + 脚本执行，走的是 `subprocess.run` 但不是直接的 `claude -p` 调用）——是否属于本次范围，留作 + §开放问题，不在 BRD 阶段预设结论（该模块已随 work-graph-node-execution-deblackbox 迁移逐步 + 萎缩，是否还值得投入见 SDD 阶段判断）。 +- **异常路由层**（`src/scheduler.py::_BLOCKED_REASON_FOR_EXCEPTION` 注册表）：这是本仓库现有的 + "异常类型 → `blocked_reason` 短码"既定映射机制（budget_guardrail、`ConsistencyCheckFailedError`、 + `UsageLimitWaitTimeoutError` 等都走这条路）。本次修复应该复用这个既有机制新增一条"真卡死"异常 + 类型 → `agent_call_stalled`，而不是发明一条新的旁路判断/上报路径。 +- **审计轨迹**（`_block_task()` 既有的 `execution_event` + `detail` 记录机制）：新增的卡死判定 + 需要沿用这条既有轨迹，`detail`/`evidence` 内容应该是"最后一次活跃信号的时间戳 + 距今间隔"这类 + 可回放证据，而不是一句笼统的错误字符串——这是复用而不是新造一套证据格式。 +- 存活信号的具体来源、判断阈值、以及是否所有调用点统一取值，是本文档刻意留给 SDD 阶段先调研 + 再决定的核心技术问题（见需求文档 §7、本 BRD §开放问题）——BRD 阶段不预设"改成流式输出"或 + "监控进程 CPU/IO"或"复用 `execution_log_file`"中的任何一个是最终方案。 + +--- + +## C. WHAT（可观察结果，不写实现） + +1. 一次总耗时明显超过当前 300/1200 秒固定值的真实 agent 调用，只要过程中持续存在可观察的活跃 + 信号，能正常跑到完成，不会被系统杀掉/判定为超时。 +2. 一次调用在启动后很快就不再有任何活跃信号、且没有报错（模拟进程真正挂起）时，系统能在有界、 + 合理的时间内判定为 `agent_call_stalled`，不会无限期挂起等待。 +3. `task.blocked_reason` 中，"真卡死"（`agent_call_stalled`）与其它未分类异常 + （`execution_error`）在数据层面是可区分的两个值，不再混为一谈——排查/未来 Boss Agent 仲裁时 + 可以直接按这个字段分流处理。 +4. 今天实际复现的 `bfd3ee82` 场景（大 prompt、Validator Work Graph 规划调用）用修复后的机制 + 重放，不再仅因为总耗时超过 300 秒就被判定异常。 + +--- + +## Scope + +### Scope In +- `trigger_org_node`/`trigger_org_node_structured` 的超时判断逻辑本身（"总时长封顶" → + "活跃度/心跳"）。 +- 各既有调用点（`e2e_pipeline.py`/`work_graph_plan.py`/`work_graph_orchestrate.py`/ + `org_level_loop.py`/`root_cause_trace.py`/`org_node_lifecycle.py`）当前固定 `timeout=300/600/ + 1200` 等参数值的语义重新定义（改成心跳判断后，这些数字的含义、以及是否需要重新核对合理性）。 +- 新增专属 `blocked_reason` 分类（如 `agent_call_stalled`），复用 `scheduler.py` 既有的异常路由 + 注册表机制接入。 + +### Scope Out（本 BRD 明确不裁定） +- **具体活跃信号选什么**（增量输出格式 / 进程 CPU-IO 活动 / 复用 `execution_log_file` 旁路等） + ——留给 SDD 阶段先做真实技术调研（见需求文档 §3.1、§7；`claude -p --output-format json` 目前 + 是一次性输出，不是增量流，这是需要先验证清楚、不能假设的事实）。 +- **`claude` CLI 调用方式的既定约定**（`--agent`/`--output-format json`/`--permission-mode + auto`，CLAUDE.md"硬约束"一节）——如果心跳检测需要切到某种增量输出格式，必须先做与现有 + ```` ```json ```` fence 提取约定的真实兼容性验证（05 §5.4.1.1 已有先例：`--agent` + + `--json-schema` 会静默丢失 `structured_output`，同类陷阱不能靠假设排除）。 +- **`p_usage_limit_wait`（`UsageLimitWaitTimeoutError`）机制**——已知、已分类的账号用量限额等待 + 重试逻辑，跟"普通调用要跑多久才算异常"是两个不同问题，不合并。 +- **`work_graph_execute.py::execute_work_graph`（Dynamic Workflow 脚本执行）是否需要同样改造** + ——列为开放问题，不预设范围内/外。 +- **Boss Agent 的自主仲裁逻辑**（另一独立 Goal，`docs/todo/boss-agent-autonomous-blocked-task- + arbiter-requirements.md`）——本 BRD 只负责让进入 `blocked_pending_human` 的信号更干净，不 + 负责任务卡住之后怎么处理。 + +--- + +## D. 组织契约（轻量版，说明原因） + +本次修复不新增 Work 节点/边，因此不套用完整的"node_id/owner/input_ref/output_schema"节点契约 +模板；改动落在既有节点 `trigger_org_node`/`trigger_org_node_structured` 内部： + +- **节点边界不变**：调用方（`e2e_pipeline.py` 等）的调用签名（`org_node`, `prompt`, + `target_folder_path`, `timeout=...`）预期保持兼容，`timeout` 参数语义变化（见 Scope In）需要 + 在 SDD 阶段明确写清楚新旧调用方是否都要跟着改。 +- **失败态契约不变**：仍然是"抛出异常 → `scheduler._blocked_reason_for` 分类 → `_block_task` + 写 `task.status='blocked_pending_human'` + `blocked_reason` + `execution_event`"这条既有链路， + 新增的只是异常类型和 `blocked_reason` 取值，不改变链路本身。 +- **证据字段**：新增判定需要保留"最后一次活跃信号时间戳"、"距今间隔"、"判定发生时刻"这三类可回放 + 证据，写入 `_block_task` 的 `detail` 参数（沿用既有字段，不新增表/列——已核实 `schema.py` 中 + `blocked_reason` 是无 CHECK 约束的 TEXT 字段，新增取值不需要 schema 迁移）。 + +--- + +## 验收标准（3 类，与需求文档 §5 一致，补充说明可测方式） + +1. **目标态（成功）**：构造一次总耗时明显超过当前 300 秒固定值、但过程中持续有可观察活跃信号的 + 真实调用，验证不会被判定为超时/卡死，能正常跑完并返回结果。 +2. **边界态（真卡死）**：构造一次启动后很快不再有任何活跃信号、且无报错的调用（模拟进程真正挂起）， + 验证系统能在合理、有界的时间内判定为 `agent_call_stalled` 并正确写入 + `blocked_pending_human`/`execution_event`，不会无限期等待。 +3. **失败态/回归**：`bfd3ee82` 的真实场景（大 prompt、Validator Work Graph 规划调用）用修复后的 + 机制重跑，验证不再因总耗时超过 300 秒被杀；同时验证旧的固定超时判断路径（`execution_error` + 兜底）不再错误吞掉这类场景。 + +--- + +## 风险 + +1. **信号本身不可靠**：如果最终选择的"活跃信号"有延迟、或某些真实卡死场景恰好仍在产生噪声信号， + 可能导致真正卡死的调用永远无法被判定为异常、任务无限挂起——这跟"固定超时太激进、误杀正常调用" + 是同一个问题的另一个极端。SDD/验收阶段必须两头都验证（见验收标准 1、2），不能只解决"误杀"这 + 一半。 +2. **调用点覆盖不全**：本次修复涉及六个模块的多个调用点，如果只改了 `org_node_trigger.py` 内部 + 逻辑但没有同步核对所有调用点传入的固定 `timeout` 语义是否已经切换成心跳含义，会出现"部分调用 + 点已修复、部分调用点仍是旧语义"的不一致状态，需要在 SDD 阶段给出完整调用点清单并逐一确认 + （已核实的调用点清单见本 BRD (e) 小节）。 +3. **`execute_work_graph` 是否同源问题**：如果 SDD 阶段判定 `work_graph_execute.py` 的固定超时 + 也需要同样改造但被漏掉，会留下另一个未被本次修复覆盖、但同样会被同类问题命中的入口——需要在 + SDD 阶段显式给出"是/否包含"的结论，不能默认排除。 + +--- + +## 阻塞未知（抄自需求文档 §7，作为 BRD 级 open questions，PL 不在本阶段自行裁定） + +- [ ] **"活跃信号"具体是什么**：`claude -p ... --output-format json` 目前是一次性输出，不是增量 + 流；需要先调研 `claude` CLI 是否有增量/流式输出格式可用（若有，切换格式前必须先做真实兼容性 + 验证，见 Scope Out 第二条），若没有，是否可以用进程级 CPU/IO 活动作为替代信号，或依赖 + `work_node_run.py`/`execution_log_file` 这条已有的实时日志旁路（前提是这次调用本身会写 + 日志）。这是本文档最需要先调研清楚的技术问题，留给 SDD 阶段。 +- [ ] **"长时间无活跃信号"的具体阈值定多少**：不再是"总时长 N 秒"，而是"距上次活跃信号 N 秒"， + 这个新的 N 该怎么定，是否所有调用点统一取值，还是按调用点历史 timeout 值（180/300/600/1200） + 折算出不同的心跳阈值。 +- [ ] **`work_graph_execute.py::execute_work_graph` 是否同属本次范围**：见风险 3，需要 SDD 阶段 + 显式给出结论。 + +--- + +## 与 Boss Agent Goal 的关系（不重复对方文档，只记录顺序结论） + +`docs/todo/boss-agent-autonomous-blocked-task-arbiter-requirements.md` 是一个强关联但独立的 +Goal：本文档解决"正常调用被误杀成 `blocked_pending_human`"，Boss Agent 解决"任务真的卡在 +`blocked_pending_human` 之后怎么自主处理"。两者可以独立立项、独立验收，但建议本文档优先落地—— +减少 `blocked_pending_human` 中的"假异常"占比，能显著降低 Boss Agent 未来需要处理的噪音信号量。 + +--- + +## PL 自审（失败关闭检查） + +- [x] WHY 包含 (a)(b)(c)(d)(e)，均可追溯到需求文档、真实代码位置、真实事件（`bfd3ee82`）或本 + Agent 自身 memory 记录，无凭空断言。 +- [x] HOW 以机制描述为主，未写具体实现代码；明确说明本次不新增 Graph 节点/边，并给出原因。 +- [x] WHAT 均为可观察结果，不描述内部函数调用细节。 +- [x] Scope In / Scope Out 显式区分，阻塞未知单独列出，未混入"事实"或"设计决策"。 +- [x] 涉及的所有调用点、异常路由机制、字段是否有约束（`blocked_reason` 是否有 CHECK）均已读取 + 实际代码核实，未凭空假设。 +- [x] 阻塞未知（存活信号具体选型、阈值、`execute_work_graph` 是否同源）均未在 BRD 阶段被强行裁定， + 明确留给 SDD 阶段先调研。 +- [x] 与既有关联 Goal（Boss Agent）的边界和顺序已说明，未重复对方范围。 +- **结论：可立项**，进入 PRD/SDD 阶段；SDD 阶段必须先完成 §开放问题第一条的真实技术调研，再定 + 具体方案，不能假设某个信号方案天然可行。 diff --git a/supervisor/deploy_stage.py b/supervisor/deploy_stage.py index 9c1803a..182379a 100644 --- a/supervisor/deploy_stage.py +++ b/supervisor/deploy_stage.py @@ -28,6 +28,7 @@ staging and cutover would otherwise lose track of it entirely. import os import shutil import socket +import sqlite3 import subprocess import sys import uuid @@ -42,6 +43,34 @@ class CheckoutFailedError(Exception): time this is raised.""" +class ConcurrentDeploymentError(Exception): + """CONCURRENT_DEPLOYMENT -- ux_deployment_active unique index detected an + existing deployment row not yet in a terminal state (finalized/ + rolled_back/blocked_pending_human) when attempting to INSERT a new one. + Unlike CheckoutFailedError, the INSERT never committed, so there is no + half-finished deployment row to mark blocked_pending_human -- callers + (the Deploy Supervisor polling loop) should treat this as 'skip this + round, retry next poll', not as a failure needing human intervention. + Diagnostic fields are real attributes, not just baked into the message: + - git_commit_hash: the commit hash this rejected call attempted to stage. + - blocking_deployment_id / blocking_status: the deployment row currently + holding the slot (if it could be located via a best-effort read-only + query); both None if it could not be located -- the exception is + still raised in that case. + """ + + def __init__(self, git_commit_hash, blocking_deployment_id=None, blocking_status=None): + self.git_commit_hash = git_commit_hash + self.blocking_deployment_id = blocking_deployment_id + self.blocking_status = blocking_status + message = "cannot stage commit {}: a deployment is already in progress".format(git_commit_hash) + if blocking_deployment_id is not None: + message += " (blocking deployment_id={}, status={})".format( + blocking_deployment_id, blocking_status + ) + super().__init__(message) + + def _last_finalized_deployment(conn): """Returns (git_commit_hash, candidate_instance_dir) of the most recent `finalized` deployment, or None if there isn't one yet (the bootstrap @@ -145,6 +174,37 @@ def _launch_candidate(candidate_dir, port, db_path, python_executable=None): ) +def _is_concurrent_deployment_conflict(error): + """Distinguish a ux_deployment_active unique-index conflict from any + other IntegrityError. sqlite reports named UNIQUE INDEX conflicts with + the index name embedded in the message, e.g. "UNIQUE constraint + failed: index 'ux_deployment_active'". Match only this exact index + name string -- do not broadly catch all IntegrityErrors, or unrelated + integrity errors would be misclassified and masked as concurrency + conflicts. + """ + return "ux_deployment_active" in str(error) + + +def _find_blocking_deployment(conn): + """Read-only diagnostic query: find the non-terminal deployment row + currently occupying the ux_deployment_active slot. Only called after a + conflict has already been detected, purely to enrich the exception. If + the query itself fails (e.g. a broken connection), swallow the + sqlite3.Error and return None -- the caller still raises + ConcurrentDeploymentError, just without blocking_deployment_id/status. + """ + try: + row = conn.execute( + "SELECT deployment_id, status FROM deployment " + "WHERE status NOT IN ('finalized', 'rolled_back', 'blocked_pending_human') " + "ORDER BY rowid DESC LIMIT 1" + ).fetchone() + return tuple(row) if row else None + except sqlite3.Error: + return None + + def stage_deployment( conn, git_commit_hash, repo_root=None, prod_db_path=None, checkout_parent_dir=None, port=None, python_executable=None, @@ -167,13 +227,25 @@ def stage_deployment( previous_commit_hash = _current_commit(repo_root) previous_instance_dir = repo_root - conn.execute( - "INSERT INTO deployment " - "(deployment_id, git_commit_hash, previous_commit_hash, status, previous_instance_dir, started_at) " - "VALUES (?, ?, ?, 'staged', ?, CURRENT_TIMESTAMP)", - (deployment_id, git_commit_hash, previous_commit_hash, previous_instance_dir), - ) - conn.commit() + try: + conn.execute( + "INSERT INTO deployment " + "(deployment_id, git_commit_hash, previous_commit_hash, status, previous_instance_dir, started_at) " + "VALUES (?, ?, ?, 'staged', ?, CURRENT_TIMESTAMP)", + (deployment_id, git_commit_hash, previous_commit_hash, previous_instance_dir), + ) + conn.commit() + except sqlite3.IntegrityError as e: + conn.rollback() + if not _is_concurrent_deployment_conflict(e): + raise + blocking = _find_blocking_deployment(conn) + blocking_deployment_id, blocking_status = blocking if blocking else (None, None) + raise ConcurrentDeploymentError( + git_commit_hash=git_commit_hash, + blocking_deployment_id=blocking_deployment_id, + blocking_status=blocking_status, + ) from e try: candidate_dir = _checkout_candidate(repo_root, git_commit_hash, deployment_id, checkout_parent_dir) diff --git a/supervisor/test_deploy_stage.py b/supervisor/test_deploy_stage.py index 57ed7f6..a13f5d4 100644 --- a/supervisor/test_deploy_stage.py +++ b/supervisor/test_deploy_stage.py @@ -9,6 +9,7 @@ Run: python3 -m unittest discover -s supervisor -p "test_*.py" -v import os import shutil +import sqlite3 import subprocess import sys import tempfile @@ -171,13 +172,19 @@ class StageDeploymentHappyPathTest(DeployStageTestBase): def test_only_one_non_terminal_deployment_allowed(self): with patch.object(deploy_stage, "_launch_candidate") as mock_launch: mock_launch.return_value.pid = 1 - deploy_stage.stage_deployment( + first = deploy_stage.stage_deployment( self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9001, ) - with self.assertRaises(Exception): # schema's ux_deployment_active partial unique index + with self.assertRaises(deploy_stage.ConcurrentDeploymentError) as ctx: deploy_stage.stage_deployment( self.conn, self.commit_v1, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9002, ) + err = ctx.exception + self.assertEqual(err.git_commit_hash, self.commit_v1) + self.assertEqual(err.blocking_deployment_id, first["deployment_id"]) + self.assertEqual(err.blocking_status, "candidate_starting") + count = self.conn.execute("SELECT COUNT(*) FROM deployment").fetchone()[0] + self.assertEqual(count, 1) class CheckoutFailedTest(DeployStageTestBase): @@ -198,6 +205,125 @@ class CheckoutFailedTest(DeployStageTestBase): mock_launch.assert_not_called() +class ConcurrentDeploymentTest(DeployStageTestBase): + """Tests for `ConcurrentDeploymentError` -- the diagnosable replacement + for the raw `sqlite3.IntegrityError` `ux_deployment_active` used to let + escape uncaught.""" + + # Every deployment.status value from src/schema.py's CHECK constraint + # that is NOT in ux_deployment_active's exclusion set (finalized/ + # rolled_back/blocked_pending_human) -- each of these must block a new + # stage_deployment() call, including the less-obvious terminal-looking + # ones (health_check_failed, rollback_failed) that are NOT actually + # excluded by the index. + _NON_TERMINAL_STATUSES = [ + "staged", "candidate_starting", "health_check_running", "health_check_passed", + "health_check_failed", "cutover_in_progress", "live", "monitoring", + "anomaly_detected", "rollback_in_progress", "rollback_failed", "monitoring_window_clean", + ] + + def _seed_deployment_row(self, deployment_id, status): + self.conn.execute( + "INSERT INTO deployment (deployment_id, git_commit_hash, previous_commit_hash, status, " + "candidate_instance_dir, previous_instance_dir) " + "VALUES (?, 'seed-commit', 'seed-previous-commit', ?, '/seed/candidate/dir', '/seed/previous/dir')", + (deployment_id, status), + ) + self.conn.commit() + + def test_rolled_back_prior_deployment_allows_new_staging(self): + self._seed_deployment_row("dep-rolled-back", "rolled_back") + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: + mock_launch.return_value.pid = 1 + result = deploy_stage.stage_deployment( + self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9010, + ) + self.assertEqual(result["status"], "candidate_starting") + row = self.conn.execute( + "SELECT status FROM deployment WHERE deployment_id = ?", (result["deployment_id"],) + ).fetchone() + self.assertEqual(row[0], "candidate_starting") + + def test_blocked_pending_human_prior_deployment_allows_new_staging(self): + self._seed_deployment_row("dep-blocked", "blocked_pending_human") + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: + mock_launch.return_value.pid = 1 + result = deploy_stage.stage_deployment( + self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9011, + ) + self.assertEqual(result["status"], "candidate_starting") + row = self.conn.execute( + "SELECT status FROM deployment WHERE deployment_id = ?", (result["deployment_id"],) + ).fetchone() + self.assertEqual(row[0], "candidate_starting") + + def test_is_concurrent_deployment_conflict_matches_only_the_exact_index_name(self): + self.assertTrue( + deploy_stage._is_concurrent_deployment_conflict( + sqlite3.IntegrityError("UNIQUE constraint failed: index 'ux_deployment_active'") + ) + ) + self.assertFalse( + deploy_stage._is_concurrent_deployment_conflict( + sqlite3.IntegrityError("NOT NULL constraint failed: deployment.git_commit_hash") + ) + ) + self.assertFalse( + deploy_stage._is_concurrent_deployment_conflict( + sqlite3.IntegrityError("UNIQUE constraint failed: index 'ux_some_other_index'") + ) + ) + + def test_find_blocking_deployment_returns_most_recently_inserted_by_rowid(self): + # ux_deployment_active itself forbids two non-terminal rows from + # coexisting (that's the whole point of this fix) -- drop it just + # for this test so two can be seeded directly, to isolate and prove + # _find_blocking_deployment's own "most recent by rowid" ordering + # logic independent of that constraint. + self.conn.execute("DROP INDEX ux_deployment_active") + self._seed_deployment_row("dep-older", "staged") + self._seed_deployment_row("dep-newer", "candidate_starting") + blocking = deploy_stage._find_blocking_deployment(self.conn) + self.assertEqual(blocking, ("dep-newer", "candidate_starting")) + + def test_find_blocking_deployment_returns_none_on_broken_connection(self): + broken_conn = schema.connect(":memory:") + broken_conn.close() + self.assertIsNone(deploy_stage._find_blocking_deployment(broken_conn)) + + def test_conn_still_usable_after_conflict_is_caught(self): + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: + mock_launch.return_value.pid = 1 + deploy_stage.stage_deployment( + self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9012, + ) + with self.assertRaises(deploy_stage.ConcurrentDeploymentError): + deploy_stage.stage_deployment( + self.conn, self.commit_v1, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9013, + ) + # conn.rollback() in the except branch must clear the dangling + # implicit transaction -- a subsequent trivial query must not raise. + self.assertEqual(self.conn.execute("SELECT 1").fetchone()[0], 1) + + def test_every_non_excluded_status_blocks_staging_with_concurrent_deployment_error(self): + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: + for status in self._NON_TERMINAL_STATUSES: + deployment_id = "dep-status-{}".format(status) + self._seed_deployment_row(deployment_id, status) + with self.assertRaises(deploy_stage.ConcurrentDeploymentError, msg=status) as ctx: + deploy_stage.stage_deployment( + self.conn, self.commit_v2, repo_root=self.repo_dir, + prod_db_path=self.prod_db_path, port=9020, + ) + self.assertEqual(ctx.exception.blocking_deployment_id, deployment_id, status) + self.assertEqual(ctx.exception.blocking_status, status, status) + mock_launch.assert_not_called() + count = self.conn.execute("SELECT COUNT(*) FROM deployment").fetchone()[0] + self.assertEqual(count, 1, status) + self.conn.execute("DELETE FROM deployment WHERE deployment_id = ?", (deployment_id,)) + self.conn.commit() + + class LaunchCandidateArgsTest(unittest.TestCase): """2026-08-06 (task_reconciliation): `_launch_candidate`'s `db_path` is always a COPY of the real production db -- without `--disable-